Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add utstring_release #95

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add utstring_release #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78888bb
Add utstring_release
tbeu Nov 25, 2016
a4f1104
Add utstring_adopt and utstring_disown macros
tbeu Nov 30, 2016
7006e53
Fix utstring_disown
tbeu Dec 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions doc/utstring.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,18 @@ Operations
| utstring_new(s) | allocate a new utstring
| utstring_renew(s) | allocate a new utstring (if s is `NULL`) otherwise clears it
| utstring_free(s) | free an allocated utstring
| utstring_init(s) | init a utstring (non-alloc)
| utstring_done(s) | dispose of a utstring (non-allocd)
| utstring_init(s) | init a utstring (non-alloc)
| utstring_done(s) | dispose of a utstring (non-alloc)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll take these diffs to the existing docs upstream right now (hopefully in a way that merges cleanly with this PR).

| utstring_printf(s,fmt,...) | printf into a utstring (appends)
| utstring_bincpy(s,bin,len) | insert binary data of length len (appends)
| utstring_concat(dst,src) | concatenate src utstring to end of dst utstring
| utstring_clear(s) | clear the content of s (setting its length to 0)
| utstring_len(s) | obtain the length of s as an unsigned integer
| utstring_body(s) | get `char*` to body of s (buffer is always null-terminated)
| utstring_adopt(s,bin,len) | init a utstring with binary data of length len (shallow copy)
| utstring_disown(s,bin) | get `char*` to body of s and clear the content of s
| utstring_find(s,pos,str,len) | forward search from pos for a substring
| utstring_findR(s,pos,str,len) | reverse search from pos a substring
| utstring_findR(s,pos,str,len) | reverse search from pos for a substring
|===============================================================================

New/free vs. init/done
Expand Down
19 changes: 19 additions & 0 deletions src/utstring.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,25 @@ do { \

#define utstring_body(s) ((s)->d)

#define utstring_adopt(s,b,l) \
do { \
if (s) { \
(s)->d = (b); \
(s)->n = (l); \
(s)->i = (l); \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@troydhanson, in the codebases where you've needed this function in the past, would this have sufficed? Or should it be utstring_adopt(s,d,i,n) for cases where the initial capacity is intentionally greater than the initial length?
I guess since C's realloc and free don't need to know the original capacity, this version is safe; but if someone later filed a feature request to make utstring work with ptr = sizedmalloc(sz) and sizedfree(ptr,sz) the way uthash currently does, it would be hard to retrofit that into this "length-only" model.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs a tweak. The other "put" functions (utstring_printf, utstring_bincpy) internally set the internal buffer length n one larger than the input size, to accommodate a trailing nul, and then forcibly put it there. (Look at the utstring_bincpy definition to see what I mean).
It's a convenience so there is a guaranteed nul terminator in the spare capacity part of the string. It is a quirk of utstring that I like because, it makes it compatible with printf (etc) that expect nul-terminated strings, while keeping the nul past the length i means that the content/length are unharmed.

One way to preserve that notion, is to switch adopt to a two parameter version utstring_adopt(s,d) that demands a C string as input, and then its implementation sets

s->i = strlen(d);
s->n = s->i +1 ;

The problem with the current three-parameter adopt is that the caller can pass a non-terminated string, breaking the expectation that utstring always has a nul after the buffer.

We can switch to the four-parameter version utstring_adopt(s,d,i,n) but, to preserve the nul expectation, I would explicitly force a nul at d[i] (which requires a preceding realloc if i==n).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbeu: Sorry! I think what happened here was that the feedback I was awaiting on 2016-12-15 did arrive, in the form of Troy's comment on 2016-12-16. And then I never explicitly said "@tbeu please address this," but I also never really bothered (AFAIR) to figure out whether I even agreed with what Troy said, either.

I suggest that at this point, with many outdated review comments and some pieces of this patch merged already, the best thing to do would be to close this PR and (if you still want the feature merged) open a fresh PR.

Do you have any thoughts on the desirability of utstring_adopt(s, buf, len) versus utstring_adopt(s, ntbs) versus utstring_adopt(s, buf, len, cap)? I think that (at the moment) I agree with Troy that utstring_adopt(s, buf, len) is the worst of both worlds, because it would let you create a non-null-terminated utstring.

} \
} while(0)

#define utstring_disown(s,b) \
do { \
if (s) { \
(b) = (s)->d; \
(s)->d = NULL; \
(s)->i = 0; \
(s)->n = 0; \
} \
} while(0)

_UNUSED_ static void utstring_printf_va(UT_string *s, const char *fmt, va_list ap) {
int n;
va_list cp;
Expand Down