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] 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 1:

(3 comments)

I like the direction of this change, but there are some nits.

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/1//COMMIT_MSG 
Commit Message:

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/1//COMMIT_MSG@19 
PS1, Line 19: Signed-off-by: Carlos O'Donell <carlos@redhat.com>
We do not use DCO and Signed-off-by.


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

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/1/locale/programs/localedef.c@531 
PS1, Line 531: 			  _("failed to allocate space for compiled locale path"));
I think this line is too long.  I think we use xmalloc in many places already, so it would make sense to introduce xasprintf and use that consistently in the application.  (The version from support/ is not suitable in this context.)


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/303/1/locale/programs/localedef.c@567 
PS1, Line 567: 	record_verbose (stderr, _("no write permission to output path: %s"),
Would it make sense to include the output path in this message as well?



-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I28b9f680711ff00252a2cb15625b774cc58ecb9d
Gerrit-Change-Number: 303
Gerrit-PatchSet: 1
Gerrit-Owner: Carlos O'Donell <carlos@redhat.com>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Thu, 24 Oct 2019 19:33:39 +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]