On Wed, Oct 4, 2023 at 12:46 PM Todd C. Miller <todd.mil...@sudo.ws> wrote:
>
> On Wed, 04 Oct 2023 11:53:51 -0600, Todd C. Miller wrote:
>
> > Yes, this should be fixed.  One difference is that in FreeBSD,
> > __swsetup() sets errno whereas in OpenBSD we set errno in the caller
> > when cantwrite() is true.  I think it makes sense to set both errno
> > and the __SERR flag in the same place.  We just need to decide which
> > place that is :-)
>
> I decided that it is simplest to set errno and __SERR in __swsetup()
> like FreeBSD has done instead of in the callers.  This is consistent
> with how fread(3) relies on __srefill() to set both errno and the
> error flag.

yeah, love it!

(writing the _other_ missing tests was on my to-do list, so i'd only
have been back in a week or two when i noticed that putc() etc had the
same problem :-) )

> We have an additional check in __swsetup() where we return EOF on
> error that also needs to set errno and __SERR.  There is no actual
> fd in that case so I've set errno to EINVAL.
>
>  - todd
>
> Index: lib/libc/stdio/fvwrite.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/fvwrite.c,v
> retrieving revision 1.20
> diff -u -p -u -r1.20 fvwrite.c
> --- lib/libc/stdio/fvwrite.c    17 Mar 2017 16:06:33 -0000      1.20
> +++ lib/libc/stdio/fvwrite.c    4 Oct 2023 18:28:33 -0000
> @@ -34,7 +34,6 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> -#include <errno.h>
>  #include <unistd.h>
>  #include "local.h"
>  #include "fvwrite.h"
> @@ -58,10 +57,8 @@ __sfvwrite(FILE *fp, struct __suio *uio)
>         if ((len = uio->uio_resid) == 0)
>                 return (0);
>         /* make sure we can write */
> -       if (cantwrite(fp)) {
> -               errno = EBADF;
> +       if (cantwrite(fp))
>                 return (EOF);
> -       }
>
>  #define        MIN(a, b) ((a) < (b) ? (a) : (b))
>  #define        COPY(n)   (void)memcpy(fp->_p, p, n)
> Index: lib/libc/stdio/putc.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/putc.c,v
> retrieving revision 1.13
> diff -u -p -u -r1.13 putc.c
> --- lib/libc/stdio/putc.c       31 Aug 2015 02:53:57 -0000      1.13
> +++ lib/libc/stdio/putc.c       4 Oct 2023 18:28:37 -0000
> @@ -32,7 +32,6 @@
>   */
>
>  #include <stdio.h>
> -#include <errno.h>
>  #include "local.h"
>
>  /*
> @@ -43,10 +42,8 @@
>  int
>  putc_unlocked(int c, FILE *fp)
>  {
> -       if (cantwrite(fp)) {
> -               errno = EBADF;
> +       if (cantwrite(fp))
>                 return (EOF);
> -       }
>         _SET_ORIENTATION(fp, -1);
>         return (__sputc(c, fp));
>  }
> Index: lib/libc/stdio/vfprintf.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/vfprintf.c,v
> retrieving revision 1.81
> diff -u -p -u -r1.81 vfprintf.c
> --- lib/libc/stdio/vfprintf.c   8 Sep 2021 15:57:27 -0000       1.81
> +++ lib/libc/stdio/vfprintf.c   4 Oct 2023 16:40:18 -0000
> @@ -457,10 +457,8 @@ __vfprintf(FILE *fp, const char *fmt0, _
>
>         _SET_ORIENTATION(fp, -1);
>         /* sorry, fprintf(read_only_file, "") returns EOF, not 0 */
> -       if (cantwrite(fp)) {
> -               errno = EBADF;
> +       if (cantwrite(fp))
>                 return (EOF);
> -       }
>
>         /* optimise fprintf(stderr) (and other unbuffered Unix files) */
>         if ((fp->_flags & (__SNBF|__SWR|__SRW)) == (__SNBF|__SWR) &&
> Index: lib/libc/stdio/vfwprintf.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/vfwprintf.c,v
> retrieving revision 1.22
> diff -u -p -u -r1.22 vfwprintf.c
> --- lib/libc/stdio/vfwprintf.c  8 Sep 2021 15:57:27 -0000       1.22
> +++ lib/libc/stdio/vfwprintf.c  4 Oct 2023 16:40:18 -0000
> @@ -451,10 +451,8 @@ __vfwprintf(FILE * __restrict fp, const
>
>         _SET_ORIENTATION(fp, 1);
>         /* sorry, fwprintf(read_only_file, "") returns EOF, not 0 */
> -       if (cantwrite(fp)) {
> -               errno = EBADF;
> +       if (cantwrite(fp))
>                 return (EOF);
> -       }
>
>         /* optimise fwprintf(stderr) (and other unbuffered Unix files) */
>         if ((fp->_flags & (__SNBF|__SWR|__SRW)) == (__SNBF|__SWR) &&
> Index: lib/libc/stdio/wbuf.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/wbuf.c,v
> retrieving revision 1.13
> diff -u -p -u -r1.13 wbuf.c
> --- lib/libc/stdio/wbuf.c       31 Aug 2015 02:53:57 -0000      1.13
> +++ lib/libc/stdio/wbuf.c       4 Oct 2023 18:28:45 -0000
> @@ -32,7 +32,6 @@
>   */
>
>  #include <stdio.h>
> -#include <errno.h>
>  #include "local.h"
>
>  /*
> @@ -54,10 +53,8 @@ __swbuf(int c, FILE *fp)
>          * calls might wrap _w from negative to positive.
>          */
>         fp->_w = fp->_lbfsize;
> -       if (cantwrite(fp)) {
> -               errno = EBADF;
> +       if (cantwrite(fp))
>                 return (EOF);
> -       }
>         c = (unsigned char)c;
>
>         /*
> Index: lib/libc/stdio/wsetup.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/wsetup.c,v
> retrieving revision 1.7
> diff -u -p -u -r1.7 wsetup.c
> --- lib/libc/stdio/wsetup.c     8 Aug 2005 08:05:36 -0000       1.7
> +++ lib/libc/stdio/wsetup.c     4 Oct 2023 18:23:48 -0000
> @@ -31,6 +31,7 @@
>   * SUCH DAMAGE.
>   */
>
> +#include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include "local.h"
> @@ -38,7 +39,7 @@
>  /*
>   * Various output routines call wsetup to be sure it is safe to write,
>   * because either _flags does not include __SWR, or _buf is NULL.
> - * _wsetup returns 0 if OK to write, nonzero otherwise.
> + * __swsetup returns 0 if OK to write, nonzero otherwise, setting errno.
>   */
>  int
>  __swsetup(FILE *fp)
> @@ -51,8 +52,11 @@ __swsetup(FILE *fp)
>          * If we are not writing, we had better be reading and writing.
>          */
>         if ((fp->_flags & __SWR) == 0) {
> -               if ((fp->_flags & __SRW) == 0)
> +               if ((fp->_flags & __SRW) == 0) {
> +                       errno = EBADF;
> +                       fp->_flags |= __SERR;
>                         return (EOF);
> +               }
>                 if (fp->_flags & __SRD) {
>                         /* clobber any ungetc data */
>                         if (HASUB(fp))
> @@ -68,8 +72,11 @@ __swsetup(FILE *fp)
>          * Make a buffer if necessary, then set _w.
>          */
>         if (fp->_bf._base == NULL) {
> -               if ((fp->_flags & (__SSTR | __SALC)) == __SSTR)
> +               if ((fp->_flags & (__SSTR | __SALC)) == __SSTR) {
> +                       errno = EINVAL;
> +                       fp->_flags |= __SERR;
>                         return (EOF);
> +               }
>                 __smakebuf(fp);
>         }
>         if (fp->_flags & __SLBF) {

Reply via email to