> Date: Wed, 19 Jun 2024 15:17:05 +0200
> From: Otto Moerbeek <o...@drijf.net>
> 
> On Tue, Jun 18, 2024 at 10:00:20PM -0700, Collin Funk wrote:
> 
> > Hi,
> > 
> > I noticed that strmode(3) says that the first argument should be
> > mode_t. OpenBSD declares it with int which is not compatible since
> > mode_t appears to be unsigned, from what I can tell.
> > 
> > NetBSD fixed this a long time ago and FreeBSD did the same before the
> > 14.0 release.
> > 
> > Apologies for the lack of diff, I don't have access to an OpenBSD
> > machine at the moment. I think something like this would work though:
> > 
> > In sys/_types.h:
> 
> I think this snippet should be in sys/types.h.
> 
> > 
> > #ifndef _MODE_T_DEFINED_
> > #define _MODE_T_DEFINED_
> > typedef __mode_t    mode_t
> > #endif
> > 
> > and then in string.h:
> 
> This part is not going to work as string.h include machine/_types.h
> but not sys/_types.h (or sys/types.h for that matter). FreeBSD
> modified it to include sys/_types.h
> 
> > #ifndef _MODE_T_DEFINED_
> > #define _MODE_T_DEFINED_
> > typedef __mode_t    mode_t
> > #endif
> > void         strmode(mode_t, char *);
> > 
> > Thanks,
> > Collin
> > 
> 
> Additionally, the implementation in src/libn/libc/string/strmode.c
> needs to start using mode_t.
> 
> Building base now with the diff below. So far so good.
> 
> But this is more tricky you would think. Modifying string.h to include
> more could have unwanted side effects for applications.
> 
>       -Otto
> 
> Index: include/string.h
> ===================================================================
> RCS file: /home/cvs/src/include/string.h,v
> diff -u -p -r1.32 string.h
> --- include/string.h  5 Sep 2017 03:16:13 -0000       1.32
> +++ include/string.h  19 Jun 2024 13:11:42 -0000
> @@ -37,7 +37,7 @@
>  
>  #include <sys/cdefs.h>
>  #include <sys/_null.h>
> -#include <machine/_types.h>
> +#include <sys/_types.h>
>  
>  /*
>   * POSIX mandates that certain string functions not present in ISO C
> @@ -128,7 +128,11 @@ size_t    strlcat(char *, const char *, si
>               __attribute__ ((__bounded__(__string__,1,3)));
>  size_t        strlcpy(char *, const char *, size_t)
>               __attribute__ ((__bounded__(__string__,1,3)));
> -void  strmode(int, char *);
> +#ifndef _MODE_T_DEFINED_
> +#define _MODE_T_DEFINED_
> +typedef __mode_t     mode_t;
> +#endif

It may be safer to drop this bit...

> +void  strmode(mode_t, char *);

...and use __mode_t in the prototype and implementation.

>  char *strsep(char **, const char *);
>  int   timingsafe_bcmp(const void *, const void *, size_t);
>  int   timingsafe_memcmp(const void *, const void *, size_t);
> Index: lib/libc/string/strmode.c
> ===================================================================
> RCS file: /home/cvs/src/lib/libc/string/strmode.c,v
> diff -u -p -r1.8 strmode.c
> --- lib/libc/string/strmode.c 31 Aug 2015 02:53:57 -0000      1.8
> +++ lib/libc/string/strmode.c 19 Jun 2024 13:11:42 -0000
> @@ -32,10 +32,8 @@
>  #include <sys/stat.h>
>  #include <string.h>
>  
> -/* XXX mode should be mode_t */
> -
>  void
> -strmode(int mode, char *p)
> +strmode(mode_t mode, char *p)
>  {
>        /* print type */
>       switch (mode & S_IFMT) {
> Index: sys/sys/types.h
> ===================================================================
> RCS file: /home/cvs/src/sys/sys/types.h,v
> diff -u -p -r1.49 types.h
> --- sys/sys/types.h   6 Aug 2022 13:31:13 -0000       1.49
> +++ sys/sys/types.h   19 Jun 2024 13:11:43 -0000
> @@ -140,7 +140,10 @@ typedef  __gid_t         gid_t;          /* group id */
>  typedef      __id_t          id_t;           /* may contain pid, uid or gid 
> */
>  typedef      __ino_t         ino_t;          /* inode number */
>  typedef      __key_t         key_t;          /* IPC key (for Sys V IPC) */
> +#ifndef _MODE_T_DEFINED_
> +#define _MODE_T_DEFINED_
>  typedef      __mode_t        mode_t;         /* permissions */
> +#endif
>  typedef      __nlink_t       nlink_t;        /* link count */
>  typedef      __rlim_t        rlim_t;         /* resource limit */
>  typedef      __segsz_t       segsz_t;        /* segment size */
> 
> 

Reply via email to