On Oct 13, 2021, at 9:02 AM, Rich Felker wrote: > > On Wed, Oct 13, 2021 at 01:21:31AM +0000, (GalaxyMaster) wrote: >> Hello, >> >> I am observing the following on musl and I am not sure that this is the way it >> should be: >> === >> galaxy@archlinux:~/musl-tests $ cat fput-to-readonly.c >> #include >> #include >> >> int main() { >> FILE *f; >> int i = 0; >> f = fopen("fput-to-readonly.c", "r"); >> errno = 0; >> i = fputs("should not be written", f); >> printf("i = %d (should be negative [EOF = %d])\n", i, EOF); >> printf("errno = %d\n", errno); >> return 0; >> } >> galaxy@archlinux:~/musl-tests $ gcc -o fput-to-readonly fput-to-readonly.c >> galaxy@archlinux:~/musl-tests $ ./fput-to-readonly >> i = -1 (should be negative [EOF = -1]) >> errno = 0 >> galaxy@archlinux:~/musl-tests $ >> === >> >> Logically, I would expect the errno variable to be set to something since there >> was clearly an error and the data has not been written to the destination. >> >> Glibc returns EBADF (9) in this case: >> === >> [galaxy@archlinux musl-tests]$ ./fput-to-readonly >> i = -1 (should be negative [EOF = -1]) >> errno = 9 >> [galaxy@archlinux musl-tests]$ >> === >> >> Should not we do the same? It kind of makes sense since the descriptor we are >> asked to write to is read-only. I think it would be just one line added to >> src/stdio/__towrite.c, something like: >> === >> --- musl-b76f37fd5625d038141b52184956fb4b7838e9a5.orig/src/stdio/__towrite.c 2021-09-24 00:09:22.000000000 +0000 >> +++ musl-b76f37fd5625d038141b52184956fb4b7838e9a5/src/stdio/__towrite.c 2021-10-13 01:16:04.713069382 +0000 >> @@ -5,6 +5,7 @@ int __towrite(FILE *f) >> f->mode |= f->mode-1; >> if (f->flags & F_NOWR) { >> f->flags |= F_ERR; >> + errno = EBADF; >> return EOF; >> } >> /* Clear read buffer (easier than summoning nasal demons) */ >> === > > This is a duplicate of: > https://www.openwall.com/lists/musl/2020/10/08/1 > > As noted then, it's undefined behavior to call stdio functions on a > stream that's not the appropriate type. We do the check quoted above > to avoid blowing up and corrupting buffer state by trying to do the > wrong type of operation, but don't set an errno because there isn't > one specified for this (since it's UB). Arguably it would be better > and more consistent with what we do elsewhere to crash, but for > whatever reason that wasn't done. > > EBADF is specified for when the underlying fd is in the wrong mode, > which is a different condition (and only can happen when you inherited > it as stdin/out/err, used fdopen, or dup2'd over an existing FILE's > fd). > > Rich Hi there, The last (relevant) message in the thread of the duplicate, at https://www.openwall.com/lists/musl/2020/10/08/3, mentions a potential fix by setting errno in __towrite()/__toread(). For clarity, would that be something acceptable? I doubt it, but was just curious, since no one replied to that suggestion either way. Best, -arw -- A. Wilcox (Sent from my iPhone) Mac, iOS, Linux software engineer