This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[review v2] localedef: Add verbose messages for failure paths.


Florian Weimer has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303
......................................................................


Patch Set 2:

(4 comments)

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/2/include/programs/xasprintf.h 
File include/programs/xasprintf.h:

PS2: 
Should this go into xmalloc.h?


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/2/include/programs/xasprintf.h@21 
PS2, Line 21: extern int xasprintf (char **string_ptr, const char *format, ...);
This needs a format attribute.  The prototype in gnulib (and support/) looks like this:

char *xasprintf (const char *format, ...);

I think we should follow that, even though we actually use the returned length in the code today.


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/2/locale/programs/localedef.c 
File locale/programs/localedef.c:

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/2/locale/programs/localedef.c@446 
PS2, Line 446:       return cp;
Can you fix the memory leak for tp, too?


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/2/locale/programs/localedef.c@559 
PS2, Line 559:   *endp++ = '/';
Maybe it would be clearer to add the slash with another xasprintf call at the end, not do the fancy in-place manipulation of the string.



-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I28b9f680711ff00252a2cb15625b774cc58ecb9d
Gerrit-Change-Number: 303
Gerrit-PatchSet: 2
Gerrit-Owner: Carlos O'Donell <carlos@redhat.com>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Fri, 25 Oct 2019 06:42:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]