[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LNX.2.20.1602211222470.21162@monopod.intra.ispras.ru>
Date: Sun, 21 Feb 2016 12:51:30 +0300 (MSK)
From: Alexander Monakov <amonakov@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] slim down and avoid undefined behavior in
unsetenv
... aaand the OCD kicks in :)
On Sat, 20 Feb 2016, Alexander Monakov wrote:
> diff --git a/src/env/unsetenv.c b/src/env/unsetenv.c
> index 3569335..f0f369f 100644
> --- a/src/env/unsetenv.c
> +++ b/src/env/unsetenv.c
[snip]
> size_t l = strlen(name);
>
> - if (!*name || strchr(name, '=')) {
> + if (!*name || memchr(name, '=', l)) {
Here I could have changed '!*name' to '!l' for a small cleanup as well.
> errno = EINVAL;
> return -1;
> }
Some places in musl tail-call to __syscall_ret(-Exxx) to set errno. I wonder
if it's accidental or there's a guideline for using one style or the other?
The only place I imagine the tailcall style might be undesired is sem_trywait,
where returning failure is not expected to be rare.
What do you think about a change that introduces __set_errno that accepts
positive errno and returns -1L? With that change __syscall_ret can become
return r < -4095UL ? r : __set_errno(-r);
> -again:
[snip]
> + for (char **e = __environ; *e; )
> + if (!memcmp(name, *e, l) && l[*e] == '=') {
Here the usage of memcmp requires that it scans buffers left-to-right and
stops on first mismatch. As I understand the standards do not guarantee that,
but musl's current implementation does, and is not interposable. Still, a
gotcha.
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
