Re: Glib proposal: add strlcpy and strlcat to glib



On Jun 10, 11:48am, Tim Janik wrote:
> Subject: Re: Glib proposal: add strlcpy and strlcat to glib

>>>your patch seems technically ok, so i've got mostly stylistic comments.

Good, & thanks.

Thanks for the constructive criticism.

>>>posting patches here for review is fine for now ;)
>>>owen seems to prefer incoming/ but i am more likely to review
>>>patches on gtk-devel-list.

Actually, I sent this to "incoming/" over a month ago.
I'll make nearly all the changes you've suggested, and resubmit it to
both the mailing list & "incoming/".

However, I do have a few minor questions/issues, which I think should
be easy to resolve.

>>>> +    do {
>>>> +      if ((*d++ = *s++) == '\0')
...
>>>> +        break;
>>>> +    } while (--n != 0);
>>>
>>>and put newlines after '}'

Do you really want to put a newline after "}" in a do..while loop?
That would cause it to look like this:
 }
 while (--n != 0);
which looks suspiciously like a bug, even though in context it's correct:
I'd recommend that, in a do..while loop, you want the trailing "while" to
be cuddled after the closing brace for readability purposes.

If people really prefer this, I'll do it. This is a minor style thing, but
it looks a little ugly (in this case) to me.


>>>>  #include <stdarg.h>
>>>>
>>>> +/* include stddef for the definition of size_t */
>>>> +#include <stddef.h>
>>>> +
>>>
>>>you don#t need this when you use gsize.

If you really want to use gsize, okay, but I agree with Owen Taylor:
if there's a perfectly valid standard C type, use it.  In this case, the
BSD/Solaris interfaces uses size_t, and it's not clear that I should
change it.

If the "gsize debate" isn't resolved soon, then I'd propose switching
to gsize for now, with a comment noting that the original interface
uses size_t and that if Glib deprecates "gsize" the interface should
change back.  Sound fair?


-- 

--- David A. Wheeler
    dwheeler@ida.org





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]