[2/2] avoid passing a parameter

Submitted by Frediano Ziglio on March 26, 2019, 9:36 a.m.

Details

Message ID 20190326093648.5669-2-fziglio@redhat.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Frediano Ziglio March 26, 2019, 9:36 a.m.
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(-)

Patch hide | download patch | download mbox

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;

Comments

Rich Felker March 26, 2019, 3:07 p.m.
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
Frediano Ziglio March 26, 2019, 3:22 p.m.
> 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