public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Ulf Hermann <ulf.hermann@qt.io>
To: Mike Frysinger <vapier@gentoo.org>, <elfutils-devel@sourceware.org>
Subject: Re: [PATCH] Check for existence of asprintf and vasprintf
Date: Fri, 24 Feb 2017 09:42:00 -0000	[thread overview]
Message-ID: <11ab9e08-3d39-1edd-b493-8d569f6a2cb6@qt.io> (raw)
In-Reply-To: <20170223185129.GU28432@vapier>


>> and the GNU variant is a whopping two lines of code:
>>
>> char *base = strrchr(path, '/');
>> return base ? base + 1 : (char *)path;
>
> and we get straight to an example of why your solutions don't scale.
> your replacement is wrong.  and ironically, it's wrong on Windows,
> which is the whole point of your work.  '/' is not the path sep used
> on every system out there.  it also does not properly handle systems
> (such as Windows) that have filesystem prefixes like C:\.

Both basename variants' documentations only talk about '/'. This is not 
the place where we should handle windows directory separators. We should 
either already expect forward slashes as input (with drive names as 
first path component, as e.g. msys does it), or convert them early on. 
And the implementation I've given above is the same code as in glibc.

However, I will actually prepare some patches to replace missing 
functions using gnulib rather than my own implementations.

Ulf

  reply	other threads:[~2017-02-24  9:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 12:50 Ulf Hermann
2017-02-22 14:40 ` Mike Frysinger
2017-02-22 16:01   ` Ulf Hermann
2017-02-22 16:32     ` Mike Frysinger
2017-02-22 16:45       ` Ulf Hermann
2017-02-22 17:04         ` Mike Frysinger
2017-02-22 17:43           ` Ulf Hermann
2017-02-22 18:57             ` Mike Frysinger
2017-02-23  7:27               ` John Ogness
2017-02-23  9:39               ` Ulf Hermann
2017-02-23 18:51                 ` Mike Frysinger
2017-02-24  9:42                   ` Ulf Hermann [this message]
2017-02-24 19:15                     ` Mike Frysinger
2017-02-23 10:18           ` Ulf Hermann
2017-02-23 11:04             ` Mark Wielaard
2017-02-22 16:38     ` Mark Wielaard
2017-02-23 10:46 ` Ulf Hermann

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=11ab9e08-3d39-1edd-b493-8d569f6a2cb6@qt.io \
    --to=ulf.hermann@qt.io \
    --cc=elfutils-devel@sourceware.org \
    --cc=vapier@gentoo.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).