From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] package/lshw: fix musl build
Date: Sat, 13 Aug 2016 00:41:07 +0200 [thread overview]
Message-ID: <20160812224107.GB5656@free.fr> (raw)
In-Reply-To: <70811547-4ef0-d4cc-5074-3786f51f406e@gmail.com>
Romain, Khem, All,
On 2016-08-04 21:25 -0700, Khem Raj spake thusly:
> On 8/4/16 9:37 AM, Thomas Petazzoni wrote:
> > Hello,
> >
> > On Thu, 4 Aug 2016 18:24:46 +0200, Romain Naour wrote:
> >
> >> +diff --git a/src/core/dasd.cc b/src/core/dasd.cc
> >> +index 626b8a8..18d19c3 100644
> >> +--- a/src/core/dasd.cc
> >> ++++ b/src/core/dasd.cc
> >> +@@ -4,6 +4,7 @@
> >> + #include <glob.h>
> >> + #include <string.h>
> >> + #include <fcntl.h>
> >> ++#include <libgen.h>
> >
> > Is this related?
>
> if we want to use POSIX compliant version of basename() then we need to
> include this yes.
Confirmed. ;-)
> >> + #include <unistd.h>
> >> + #include <inttypes.h>
> >> + #include <sys/ioctl.h>
> >> +@@ -42,7 +43,7 @@ bool scan_dasd(hwNode & n)
> >> + {
> >> + for(dev_num=0;dev_num<devices.gl_pathc;dev_num++)
> >> + {
> >> +- dev_name = basename(devices.gl_pathv[dev_num]);
> >> ++ dev_name = basename(const_cast<char *>(devices.gl_pathv[dev_num]));
> >
> > I'm not super familiar with C++ stuff, but why is this problem musl
> > specific? The basename() function is "char *basename(char *)"
> > regardless of the C library being used. What makes it error out with
> > musl and not with other C libraries?
>
> glibc and uclibc implement the GNU version of basename() API which does
> not modify the argument ( const char*) and when including string.h and
> not libgen.h we are saying we want gnu version. musl does not implement
> the gnu extention, it only has POSIX version. Hence you see the issue
> just on musl.
>
> So the patch makes it compile for all libcs. however, it should be
> tested on glibc/uclibc since there were bugs in the posix implementation
> of basename() see
>
> http://man7.org/linux/man-pages/man3/basename.3.html
>
> if it cant be tested then make the posix version use only for musl
> which I would not recommend, but its an option
Note however that upstream already did a similar change in an other
file (src/core/pci.cc, by the end of the commit):
https://github.com/lyonel/lshw/commit/cd690bff1516b40fecd5ec4a7f6619e5bffc3cf0
Romain did open a pull request, but it has not yet been merged:
https://github.com/lyonel/lshw/pull/17
However, I believe casting is utterly wrong: c_str() returns a
const char* and C++98 states that that string must *not* be modified,
but the POSIX basename() *does* modify its argument.
So, I really believe that the correct fix is completely different
(abstracting away basename, maybe?).
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
next prev parent reply other threads:[~2016-08-12 22:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-04 16:24 [Buildroot] [PATCH] package/lshw: fix musl build Romain Naour
2016-08-04 16:37 ` Thomas Petazzoni
2016-08-04 17:09 ` Romain Naour
2016-08-04 18:09 ` Thomas Petazzoni
2016-08-04 20:34 ` Romain Naour
2016-08-05 4:25 ` Khem Raj
2016-08-12 22:41 ` Yann E. MORIN [this message]
2016-08-13 2:32 ` Khem Raj
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160812224107.GB5656@free.fr \
--to=yann.morin.1998@free.fr \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox