| Message ID | 1514243113-6027-1-git-send-email-rosenp@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [LEDE-DEV] odhcp6c: Replace strerror(errno) with %m | expand |
Le 12/25/17 à 15:05, Rosen Penev a écrit : > Reduction of 48 bytes in compiled size. No functional difference. > > -pedantic was removed as %m is a GNU extension. My 2 cents, I really think your patches are moving us in the wrong direction, even if all C libraries that are currently supported implement this extension, this is... a GNU extension, therefore, we should stay away from it. This was mentioned before, so I will repeat it here, OpenWrt/LEDE support building with external C libraries, I spent an enormous amount of time this year fixing problems discovered doing that, these type of changes clearly make these types of effort even more of a catch up game, which is already no fun. The size savings absolutely do not warrant making such changes across the board, the cost/reward is not just worth it. If you are so obsessed with size, I am sure we can find better ways to achieve that by generalizing the use of LTO or even re-linking programs with just what they need.
On Mon, Dec 25, 2017 at 3:23 PM, Florian Fainelli <f.fainelli@gmail.com> wrote: > Le 12/25/17 à 15:05, Rosen Penev a écrit : >> Reduction of 48 bytes in compiled size. No functional difference. >> >> -pedantic was removed as %m is a GNU extension. > > My 2 cents, I really think your patches are moving us in the wrong > direction, even if all C libraries that are currently supported > implement this extension, this is... a GNU extension, therefore, we > should stay away from it. > As far as the general avoiding GNU extensions standpoint goes, I hold no particular opinion. As far as these %m patches go, BSDs such as macOS only print "m" when given %m in a format string. Meaning, they help size in the common case and hurt readability in the non-common case. I think this should be determined on a case-by-case basis. As far as odhcp6c goes, it's up to the maintainer. > This was mentioned before, so I will repeat it here, OpenWrt/LEDE > support building with external C libraries, I spent an enormous amount > of time this year fixing problems discovered doing that, these type of > changes clearly make these types of effort even more of a catch up game, > which is already no fun. > > The size savings absolutely do not warrant making such changes across > the board, the cost/reward is not just worth it. If you are so obsessed > with size, I am sure we can find better ways to achieve that by > generalizing the use of LTO or even re-linking programs with just what > they need. > -- > Florian
On Tue, Jan 2, 2018 at 11:01 PM, Rosen Penev <rosenp@gmail.com> wrote: > On Mon, Dec 25, 2017 at 3:23 PM, Florian Fainelli <f.fainelli@gmail.com> wrote: >> Le 12/25/17 à 15:05, Rosen Penev a écrit : >>> Reduction of 48 bytes in compiled size. No functional difference. >>> >>> -pedantic was removed as %m is a GNU extension. >> >> My 2 cents, I really think your patches are moving us in the wrong >> direction, even if all C libraries that are currently supported >> implement this extension, this is... a GNU extension, therefore, we >> should stay away from it. >> > As far as the general avoiding GNU extensions standpoint goes, I hold > no particular opinion. > As far as these %m patches go, BSDs such as macOS only print "m" when > given %m in a format string. Meaning, they help size in the common > case and hurt readability in the non-common case. I think this should > be determined on a case-by-case basis. > As far as odhcp6c goes, it's up to the maintainer. NAK for the patch as the code is intended to be used as "ISO C source" enforced by the pedantic compile option which is removed in this patch. Adding the pedantic compile option results into the compiler warning "error: ISO C does not support the '%m' gnu_printf format [-Werror=format=]" which results into a compilation failure due to Werror. Hans >> This was mentioned before, so I will repeat it here, OpenWrt/LEDE >> support building with external C libraries, I spent an enormous amount >> of time this year fixing problems discovered doing that, these type of >> changes clearly make these types of effort even more of a catch up game, >> which is already no fun. >> >> The size savings absolutely do not warrant making such changes across >> the board, the cost/reward is not just worth it. If you are so obsessed >> with size, I am sure we can find better ways to achieve that by >> generalizing the use of LTO or even re-linking programs with just what >> they need. >> -- >> Florian > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev
diff --git a/CMakeLists.txt b/CMakeLists.txt index a5b0cb3..c80c32a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,7 +5,7 @@ cmake_policy(SET CMP0015 NEW) project(odhcp6c C) set(CMAKE_SHARED_LIBRARY_LINK_C_FLAGS "") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -std=c99") -add_definitions(-D_GNU_SOURCE -Wall -Werror -Wextra -pedantic) +add_definitions(-D_GNU_SOURCE -Wall -Werror -Wextra) if(${EXT_PREFIX_CLASS}) add_definitions(-DEXT_PREFIX_CLASS=${EXT_PREFIX_CLASS}) diff --git a/src/dhcpv6.c b/src/dhcpv6.c index 9dce577..0944dd2 100644 --- a/src/dhcpv6.c +++ b/src/dhcpv6.c @@ -522,9 +522,9 @@ static void dhcpv6_send(enum dhcpv6_msg type, uint8_t trid[3], uint32_t ecs) if (sendmsg(sock, &msg, 0) < 0) { char in6_str[INET6_ADDRSTRLEN]; - syslog(LOG_ERR, "Failed to send DHCPV6 message to %s (%s)", + syslog(LOG_ERR, "Failed to send DHCPV6 message to %s (%m)", inet_ntop(AF_INET6, (const void *)&srv.sin6_addr, - in6_str, sizeof(in6_str)), strerror(errno)); + in6_str, sizeof(in6_str))); } } diff --git a/src/odhcp6c.c b/src/odhcp6c.c index 666af5c..a14c22e 100644 --- a/src/odhcp6c.c +++ b/src/odhcp6c.c @@ -255,15 +255,14 @@ int main(_unused int argc, char* const argv[]) init_dhcpv6(ifname, client_options, sol_timeout) || ra_init(ifname, &ifid, ra_options, ra_holdoff_interval) || script_init(script, ifname)) { - syslog(LOG_ERR, "failed to initialize: %s", strerror(errno)); + syslog(LOG_ERR, "failed to initialize: %m"); return 3; } if (daemonize) { openlog("odhcp6c", LOG_PID, LOG_DAEMON); // Disable LOG_PERROR if (daemon(0, 0)) { - syslog(LOG_ERR, "Failed to daemonize: %s", - strerror(errno)); + syslog(LOG_ERR, "Failed to daemonize: %m"); return 4; }
Reduction of 48 bytes in compiled size. No functional difference. -pedantic was removed as %m is a GNU extension. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- CMakeLists.txt | 2 +- src/dhcpv6.c | 4 ++-- src/odhcp6c.c | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-)