Skip to content

Conversation

@memreflect
Copy link
Contributor

Closes #126.

This also fixes a bug when the -1 flag was not specified.
The same error message buffer used by esstrerror() was reused for all error messages, so the same pointer was mistakenly added to the list of results multiple times:

; paths = /usr/bin /tmp/nonexistent/missing
; echo <={access -w $paths}
No such file or directory No such file or directory
; paths = $paths(2) $paths(1)
; echo <={access -w $paths}
Permission denied Permission denied

This also meant an access -1e invocation that throws an exception would somehow appear to change the error string obtained from a previous access invocation without -1:

; x = <={access -w $paths(1)}
; var x
x = 'No such file or directory'
; access -1e -w $paths(2)
Permission denied
; var x
x = 'Permission denied'

This also fixes a bug when the `-1` flag was not specified.
The return value of `esstrerror()` was not being duplicated before
adding it to the list of results, so the same error message buffer used
by `esstrerror()` was reused for all messages:

```
; x = <={access /foo/bar/missing}

; var x
x = 'No such file or directory'

; access -1e -f /
Permission denied

; var x
x = 'Permission denied'

;

; paths = /usr/bin /tmp/nonexistent/missing

; echo <={access -w $paths}
No such file or directory No such file or directory

; paths = $paths(2) $paths(1)

; echo <={access -w $paths}
Permission denied Permission denied
```
@jpco
Copy link
Collaborator

jpco commented Oct 31, 2024

Looks good to me, though I can't personally get the buggy error message behavior you describe here to happen... curious.

@memreflect
Copy link
Contributor Author

Looks good to me, though I can't personally get the buggy error message behavior you describe here to happen... curious.

On FreeBSD, the same pointer is used, while glibc seems to do something else.
It's a matter of copying the pointer or copying the content of the string, and $&access copied the pointer, so when the contents of the same pointer changed, all error strings appeared to have the same value.

@jpco
Copy link
Collaborator

jpco commented Nov 1, 2024

GCC gives me this warning:

access.c: In function ‘testperm’:
access.c:54:39: warning: comparison of integer expressions of different signedness: ‘__mode_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
   54 |         return (stat->st_mode & mask) == mask ? 0 : EACCES;
      |                                       ^~

Mind making mask unsigned? Might be worth making the perm that gets passed down the call stack unsigned, too. And maybe even the type that's in prim_access() while we're at it.

@memreflect
Copy link
Contributor Author

Mind making mask unsigned? Might be worth making the perm that gets passed down the call stack unsigned, too. And maybe even the type that's in prim_access() while we're at it.

That's what i get for daily-driving a system where sizeof (mode_t) < sizeof (int).
Yet another instance where using POSIX would have prevented such a thing. :p

All suggested changes have been applied and verified to have no impact upon the existing functionality on FreeBSD, Debian GNU/Linux, and Haiku OS.

@jpco jpco merged commit 60499ab into wryun:master Nov 2, 2024
@memreflect memreflect deleted the access-fix branch November 5, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect permission mask usage in $&access ?

2 participants