* [PATCH 1/2] use __strchrnul instead of strchr and strlen
@ 2019-03-26 9:36 Frediano Ziglio
2019-03-26 9:36 ` [PATCH 2/2] avoid passing a parameter Frediano Ziglio
2019-04-02 9:57 ` [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
0 siblings, 2 replies; 6+ messages in thread
From: Frediano Ziglio @ 2019-03-26 9:36 UTC (permalink / raw)
To: musl; +Cc: Frediano Ziglio
The result is the same but takes less code.
Note that __execvpe calls getenv which calls __strchrnul so even
using static output the size of the executable won't grow.
---
src/process/execvp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/process/execvp.c b/src/process/execvp.c
index 1fdf036f..ef3b9dd5 100644
--- a/src/process/execvp.c
+++ b/src/process/execvp.c
@@ -28,8 +28,7 @@ int __execvpe(const char *file, char *const argv[], char *const envp[])
for(p=path; ; p=z) {
char b[l+k+1];
- z = strchr(p, ':');
- if (!z) z = p+strlen(p);
+ z = __strchrnul(p, ':');
if (z-p >= l) {
if (!*z++) break;
continue;
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] avoid passing a parameter
2019-03-26 9:36 [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
@ 2019-03-26 9:36 ` Frediano Ziglio
2019-03-26 15:07 ` Rich Felker
2019-04-02 9:57 ` [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
1 sibling, 1 reply; 6+ messages in thread
From: Frediano Ziglio @ 2019-03-26 9:36 UTC (permalink / raw)
To: musl; +Cc: Frediano Ziglio
Make code slightly smaller.
"file" should not be long and it should fit in NAME_MAX so
this code would be faster only rarely.
---
src/process/execvp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/process/execvp.c b/src/process/execvp.c
index ef3b9dd5..a2726af9 100644
--- a/src/process/execvp.c
+++ b/src/process/execvp.c
@@ -19,7 +19,7 @@ int __execvpe(const char *file, char *const argv[], char *const envp[])
return execve(file, argv, envp);
if (!path) path = "/usr/local/bin:/bin:/usr/bin";
- k = strnlen(file, NAME_MAX+1);
+ k = strlen(file);
if (k > NAME_MAX) {
errno = ENAMETOOLONG;
return -1;
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] avoid passing a parameter
2019-03-26 9:36 ` [PATCH 2/2] avoid passing a parameter Frediano Ziglio
@ 2019-03-26 15:07 ` Rich Felker
2019-03-26 15:22 ` Frediano Ziglio
0 siblings, 1 reply; 6+ messages in thread
From: Rich Felker @ 2019-03-26 15:07 UTC (permalink / raw)
To: musl
On Tue, Mar 26, 2019 at 09:36:48AM +0000, Frediano Ziglio wrote:
> Make code slightly smaller.
> "file" should not be long and it should fit in NAME_MAX so
> this code would be faster only rarely.
> ---
> src/process/execvp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/process/execvp.c b/src/process/execvp.c
> index ef3b9dd5..a2726af9 100644
> --- a/src/process/execvp.c
> +++ b/src/process/execvp.c
> @@ -19,7 +19,7 @@ int __execvpe(const char *file, char *const argv[], char *const envp[])
> return execve(file, argv, envp);
>
> if (!path) path = "/usr/local/bin:/bin:/usr/bin";
> - k = strnlen(file, NAME_MAX+1);
> + k = strlen(file);
> if (k > NAME_MAX) {
> errno = ENAMETOOLONG;
> return -1;
> --
> 2.20.1
Patch 1 looks ok, but patch 2 here is contrary to the direction I want
to take things in musl. Using strnlen everywhere that excess length is
an error is an idiom I'm trying to adopt all over. Here there's no
serious practical impact either way (size difference is tiny, input
string is not untrusted), but I still prefer use of the idiom.
Rich
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] avoid passing a parameter
2019-03-26 15:07 ` Rich Felker
@ 2019-03-26 15:22 ` Frediano Ziglio
0 siblings, 0 replies; 6+ messages in thread
From: Frediano Ziglio @ 2019-03-26 15:22 UTC (permalink / raw)
To: musl
> On Tue, Mar 26, 2019 at 09:36:48AM +0000, Frediano Ziglio wrote:
> > Make code slightly smaller.
> > "file" should not be long and it should fit in NAME_MAX so
> > this code would be faster only rarely.
> > ---
> > src/process/execvp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/process/execvp.c b/src/process/execvp.c
> > index ef3b9dd5..a2726af9 100644
> > --- a/src/process/execvp.c
> > +++ b/src/process/execvp.c
> > @@ -19,7 +19,7 @@ int __execvpe(const char *file, char *const argv[], char
> > *const envp[])
> > return execve(file, argv, envp);
> >
> > if (!path) path = "/usr/local/bin:/bin:/usr/bin";
> > - k = strnlen(file, NAME_MAX+1);
> > + k = strlen(file);
> > if (k > NAME_MAX) {
> > errno = ENAMETOOLONG;
> > return -1;
> > --
> > 2.20.1
>
> Patch 1 looks ok, but patch 2 here is contrary to the direction I want
> to take things in musl. Using strnlen everywhere that excess length is
> an error is an idiom I'm trying to adopt all over. Here there's no
> serious practical impact either way (size difference is tiny, input
> string is not untrusted), but I still prefer use of the idiom.
>
> Rich
>
Fine for me to drop 2/2, I reached this code trying to workaround a
bug in binutils (I have a Fedora 29 with binutils 2.31 where each
static executable produced by musl were crashing).
Why not updating https://wiki.musl-libc.org/coding-style.html ?
Frediano
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] use __strchrnul instead of strchr and strlen
2019-03-26 9:36 [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
2019-03-26 9:36 ` [PATCH 2/2] avoid passing a parameter Frediano Ziglio
@ 2019-04-02 9:57 ` Frediano Ziglio
2019-04-02 14:40 ` Rich Felker
1 sibling, 1 reply; 6+ messages in thread
From: Frediano Ziglio @ 2019-04-02 9:57 UTC (permalink / raw)
To: musl
ping
>
> The result is the same but takes less code.
> Note that __execvpe calls getenv which calls __strchrnul so even
> using static output the size of the executable won't grow.
> ---
> src/process/execvp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/process/execvp.c b/src/process/execvp.c
> index 1fdf036f..ef3b9dd5 100644
> --- a/src/process/execvp.c
> +++ b/src/process/execvp.c
> @@ -28,8 +28,7 @@ int __execvpe(const char *file, char *const argv[], char
> *const envp[])
>
> for(p=path; ; p=z) {
> char b[l+k+1];
> - z = strchr(p, ':');
> - if (!z) z = p+strlen(p);
> + z = __strchrnul(p, ':');
> if (z-p >= l) {
> if (!*z++) break;
> continue;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] use __strchrnul instead of strchr and strlen
2019-04-02 9:57 ` [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
@ 2019-04-02 14:40 ` Rich Felker
0 siblings, 0 replies; 6+ messages in thread
From: Rich Felker @ 2019-04-02 14:40 UTC (permalink / raw)
To: musl
On Tue, Apr 02, 2019 at 05:57:33AM -0400, Frediano Ziglio wrote:
> ping
Thanks for the ping. Applying!
Rich
> > The result is the same but takes less code.
> > Note that __execvpe calls getenv which calls __strchrnul so even
> > using static output the size of the executable won't grow.
> > ---
> > src/process/execvp.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/src/process/execvp.c b/src/process/execvp.c
> > index 1fdf036f..ef3b9dd5 100644
> > --- a/src/process/execvp.c
> > +++ b/src/process/execvp.c
> > @@ -28,8 +28,7 @@ int __execvpe(const char *file, char *const argv[], char
> > *const envp[])
> >
> > for(p=path; ; p=z) {
> > char b[l+k+1];
> > - z = strchr(p, ':');
> > - if (!z) z = p+strlen(p);
> > + z = __strchrnul(p, ':');
> > if (z-p >= l) {
> > if (!*z++) break;
> > continue;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-02 14:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-26 9:36 [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
2019-03-26 9:36 ` [PATCH 2/2] avoid passing a parameter Frediano Ziglio
2019-03-26 15:07 ` Rich Felker
2019-03-26 15:22 ` Frediano Ziglio
2019-04-02 9:57 ` [PATCH 1/2] use __strchrnul instead of strchr and strlen Frediano Ziglio
2019-04-02 14:40 ` Rich Felker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
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 NNTP newsgroup(s).