ksh INT32 type

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

ksh INT32 type

Michael McConville-2
I may be totally off base here, but:

INT32's comment suggests that the configure script checks that int is >=
32 bits. However, i don't think that script's around anymore, and ANSI
specifies a minimum of only 16 bits.

The comment also says that INT32 can be 64 bits, but it's then used as
Tflag, whose comment says it can't be > 32 bits.

Maybe we should just replace it with int32_t?

I also fixed a couple adjacent comment typos.

Index: jobs.c
===================================================================
RCS file: /cvs/src/bin/ksh/jobs.c,v
retrieving revision 1.41
diff -u -p -r1.41 jobs.c
--- jobs.c 18 Apr 2015 18:28:36 -0000 1.41
+++ jobs.c 10 Sep 2015 00:06:59 -0000
@@ -71,7 +71,7 @@ struct job {
  int status; /* exit status of last process */
  pid_t pgrp; /* process group of job */
  pid_t ppid; /* pid of process that forked job */
- INT32 age; /* number of jobs started */
+ int32_t age; /* number of jobs started */
  struct timeval systime; /* system time used by job */
  struct timeval usrtime; /* user time used by job */
  Proc *proc_list; /* process list */
@@ -111,7 +111,7 @@ static Job *async_job;
 static pid_t async_pid;
 
 static int nzombie; /* # of zombies owned by this process */
-INT32 njobs; /* # of jobs started */
+int32_t njobs; /* # of jobs started */
 static int child_max; /* CHILD_MAX */
 
 
Index: sh.h
===================================================================
RCS file: /cvs/src/bin/ksh/sh.h,v
retrieving revision 1.33
diff -u -p -r1.33 sh.h
--- sh.h 18 Dec 2013 13:53:12 -0000 1.33
+++ sh.h 10 Sep 2015 00:06:59 -0000
@@ -28,12 +28,6 @@
 
 #include <paths.h>
 
-/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
- * by autoconf (assumes an 8 bit byte, but I'm not concerned).
- * NOTE: INT32 may end up being more than 32 bits.
- */
-# define INT32 int
-
 /* end of common headers */
 
 /* some useful #defines */
@@ -52,8 +46,8 @@
 #define sizeofN(type, n) (sizeof(type) * (n))
 #define BIT(i) (1<<(i)) /* define bit in flag */
 
-/* Table flag type - needs > 16 and < 32 bits */
-typedef INT32 Tflag;
+/* Table flag type - needs >= 16 and <= 32 bits */
+typedef int32_t Tflag;
 
 #define NUFILE 32 /* Number of user-accessible files */
 #define FDBASE 10 /* First file usable by Shell */
@@ -353,7 +347,7 @@ EXTERN Getopt user_opt; /* parsing stat
 
 /* This for co-processes */
 
-typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
+typedef int32_t Coproc_id; /* something that won't (realistically) wrap */
 struct coproc {
  int read; /* pipe from co-process's stdout */
  int readw; /* other side of read (saved temporarily) */

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Nicholas Marriott-2
I think all of these except perhaps Coproc_id would be better as plain
int not int32_t.

The typedefs could probably die completely (definitely Tflag anyway) but
separate diff.


On Wed, Sep 09, 2015 at 08:27:14PM -0400, Michael McConville wrote:

> I may be totally off base here, but:
>
> INT32's comment suggests that the configure script checks that int is >=
> 32 bits. However, i don't think that script's around anymore, and ANSI
> specifies a minimum of only 16 bits.
>
> The comment also says that INT32 can be 64 bits, but it's then used as
> Tflag, whose comment says it can't be > 32 bits.
>
> Maybe we should just replace it with int32_t?
>
> I also fixed a couple adjacent comment typos.
>
> Index: jobs.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/jobs.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 jobs.c
> --- jobs.c 18 Apr 2015 18:28:36 -0000 1.41
> +++ jobs.c 10 Sep 2015 00:06:59 -0000
> @@ -71,7 +71,7 @@ struct job {
>   int status; /* exit status of last process */
>   pid_t pgrp; /* process group of job */
>   pid_t ppid; /* pid of process that forked job */
> - INT32 age; /* number of jobs started */
> + int32_t age; /* number of jobs started */
>   struct timeval systime; /* system time used by job */
>   struct timeval usrtime; /* user time used by job */
>   Proc *proc_list; /* process list */
> @@ -111,7 +111,7 @@ static Job *async_job;
>  static pid_t async_pid;
>  
>  static int nzombie; /* # of zombies owned by this process */
> -INT32 njobs; /* # of jobs started */
> +int32_t njobs; /* # of jobs started */
>  static int child_max; /* CHILD_MAX */
>  
>  
> Index: sh.h
> ===================================================================
> RCS file: /cvs/src/bin/ksh/sh.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 sh.h
> --- sh.h 18 Dec 2013 13:53:12 -0000 1.33
> +++ sh.h 10 Sep 2015 00:06:59 -0000
> @@ -28,12 +28,6 @@
>  
>  #include <paths.h>
>  
> -/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
> - * by autoconf (assumes an 8 bit byte, but I'm not concerned).
> - * NOTE: INT32 may end up being more than 32 bits.
> - */
> -# define INT32 int
> -
>  /* end of common headers */
>  
>  /* some useful #defines */
> @@ -52,8 +46,8 @@
>  #define sizeofN(type, n) (sizeof(type) * (n))
>  #define BIT(i) (1<<(i)) /* define bit in flag */
>  
> -/* Table flag type - needs > 16 and < 32 bits */
> -typedef INT32 Tflag;
> +/* Table flag type - needs >= 16 and <= 32 bits */
> +typedef int32_t Tflag;
>  
>  #define NUFILE 32 /* Number of user-accessible files */
>  #define FDBASE 10 /* First file usable by Shell */
> @@ -353,7 +347,7 @@ EXTERN Getopt user_opt; /* parsing stat
>  
>  /* This for co-processes */
>  
> -typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
> +typedef int32_t Coproc_id; /* something that won't (realistically) wrap */
>  struct coproc {
>   int read; /* pipe from co-process's stdout */
>   int readw; /* other side of read (saved temporarily) */
>

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Martijn van Duren
I already sent this diff on September 1st.[1]
Pointed out in private to and ok by nicm@

[1]http://marc.info/?l=openbsd-tech&m=144112883814618&w=2

On 09/10/15 11:05, Nicholas Marriott wrote:

> I think all of these except perhaps Coproc_id would be better as plain
> int not int32_t.
>
> The typedefs could probably die completely (definitely Tflag anyway) but
> separate diff.
>
>
> On Wed, Sep 09, 2015 at 08:27:14PM -0400, Michael McConville wrote:
>> I may be totally off base here, but:
>>
>> INT32's comment suggests that the configure script checks that int is >=
>> 32 bits. However, i don't think that script's around anymore, and ANSI
>> specifies a minimum of only 16 bits.
>>
>> The comment also says that INT32 can be 64 bits, but it's then used as
>> Tflag, whose comment says it can't be > 32 bits.
>>
>> Maybe we should just replace it with int32_t?
>>
>> I also fixed a couple adjacent comment typos.
>>
>> Index: jobs.c
>> ===================================================================
>> RCS file: /cvs/src/bin/ksh/jobs.c,v
>> retrieving revision 1.41
>> diff -u -p -r1.41 jobs.c
>> --- jobs.c 18 Apr 2015 18:28:36 -0000 1.41
>> +++ jobs.c 10 Sep 2015 00:06:59 -0000
>> @@ -71,7 +71,7 @@ struct job {
>>   int status; /* exit status of last process */
>>   pid_t pgrp; /* process group of job */
>>   pid_t ppid; /* pid of process that forked job */
>> - INT32 age; /* number of jobs started */
>> + int32_t age; /* number of jobs started */
>>   struct timeval systime; /* system time used by job */
>>   struct timeval usrtime; /* user time used by job */
>>   Proc *proc_list; /* process list */
>> @@ -111,7 +111,7 @@ static Job *async_job;
>>   static pid_t async_pid;
>>
>>   static int nzombie; /* # of zombies owned by this process */
>> -INT32 njobs; /* # of jobs started */
>> +int32_t njobs; /* # of jobs started */
>>   static int child_max; /* CHILD_MAX */
>>
>>
>> Index: sh.h
>> ===================================================================
>> RCS file: /cvs/src/bin/ksh/sh.h,v
>> retrieving revision 1.33
>> diff -u -p -r1.33 sh.h
>> --- sh.h 18 Dec 2013 13:53:12 -0000 1.33
>> +++ sh.h 10 Sep 2015 00:06:59 -0000
>> @@ -28,12 +28,6 @@
>>
>>   #include <paths.h>
>>
>> -/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
>> - * by autoconf (assumes an 8 bit byte, but I'm not concerned).
>> - * NOTE: INT32 may end up being more than 32 bits.
>> - */
>> -# define INT32 int
>> -
>>   /* end of common headers */
>>
>>   /* some useful #defines */
>> @@ -52,8 +46,8 @@
>>   #define sizeofN(type, n) (sizeof(type) * (n))
>>   #define BIT(i) (1<<(i)) /* define bit in flag */
>>
>> -/* Table flag type - needs > 16 and < 32 bits */
>> -typedef INT32 Tflag;
>> +/* Table flag type - needs >= 16 and <= 32 bits */
>> +typedef int32_t Tflag;
>>
>>   #define NUFILE 32 /* Number of user-accessible files */
>>   #define FDBASE 10 /* First file usable by Shell */
>> @@ -353,7 +347,7 @@ EXTERN Getopt user_opt; /* parsing stat
>>
>>   /* This for co-processes */
>>
>> -typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
>> +typedef int32_t Coproc_id; /* something that won't (realistically) wrap */
>>   struct coproc {
>>   int read; /* pipe from co-process's stdout */
>>   int readw; /* other side of read (saved temporarily) */
>>
>

ksh_int32.diff (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Nicholas Marriott-2
It would be more helpful to post the diff again rather than a link
buried in another thread.

Any oks for this?


Index: jobs.c
===================================================================
RCS file: /cvs/src/bin/ksh/jobs.c,v
retrieving revision 1.41
diff -u -p -r1.41 jobs.c
--- jobs.c 18 Apr 2015 18:28:36 -0000 1.41
+++ jobs.c 10 Sep 2015 11:45:02 -0000
@@ -71,7 +71,7 @@ struct job {
  int status; /* exit status of last process */
  pid_t pgrp; /* process group of job */
  pid_t ppid; /* pid of process that forked job */
- INT32 age; /* number of jobs started */
+ int age; /* number of jobs started */
  struct timeval systime; /* system time used by job */
  struct timeval usrtime; /* user time used by job */
  Proc *proc_list; /* process list */
@@ -111,7 +111,7 @@ static Job *async_job;
 static pid_t async_pid;
 
 static int nzombie; /* # of zombies owned by this process */
-INT32 njobs; /* # of jobs started */
+int njobs; /* # of jobs started */
 static int child_max; /* CHILD_MAX */
 
 
Index: sh.h
===================================================================
RCS file: /cvs/src/bin/ksh/sh.h,v
retrieving revision 1.33
diff -u -p -r1.33 sh.h
--- sh.h 18 Dec 2013 13:53:12 -0000 1.33
+++ sh.h 10 Sep 2015 11:45:02 -0000
@@ -28,12 +28,6 @@
 
 #include <paths.h>
 
-/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
- * by autoconf (assumes an 8 bit byte, but I'm not concerned).
- * NOTE: INT32 may end up being more than 32 bits.
- */
-# define INT32 int
-
 /* end of common headers */
 
 /* some useful #defines */
@@ -53,7 +47,7 @@
 #define BIT(i) (1<<(i)) /* define bit in flag */
 
 /* Table flag type - needs > 16 and < 32 bits */
-typedef INT32 Tflag;
+typedef int Tflag;
 
 #define NUFILE 32 /* Number of user-accessible files */
 #define FDBASE 10 /* First file usable by Shell */
@@ -353,7 +347,7 @@ EXTERN Getopt user_opt; /* parsing stat
 
 /* This for co-processes */
 
-typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
+typedef int Coproc_id; /* something that won't (realistically) wrap */
 struct coproc {
  int read; /* pipe from co-process's stdout */
  int readw; /* other side of read (saved temporarily) */



On Thu, Sep 10, 2015 at 01:13:24PM +0200, Martijn van Duren wrote:

> I already sent this diff on September 1st.[1]
> Pointed out in private to and ok by nicm@
>
> [1]http://marc.info/?l=openbsd-tech&m=144112883814618&w=2
>
> On 09/10/15 11:05, Nicholas Marriott wrote:
> >I think all of these except perhaps Coproc_id would be better as plain
> >int not int32_t.
> >
> >The typedefs could probably die completely (definitely Tflag anyway) but
> >separate diff.
> >
> >
> >On Wed, Sep 09, 2015 at 08:27:14PM -0400, Michael McConville wrote:
> >>I may be totally off base here, but:
> >>
> >>INT32's comment suggests that the configure script checks that int is >=
> >>32 bits. However, i don't think that script's around anymore, and ANSI
> >>specifies a minimum of only 16 bits.
> >>
> >>The comment also says that INT32 can be 64 bits, but it's then used as
> >>Tflag, whose comment says it can't be > 32 bits.
> >>
> >>Maybe we should just replace it with int32_t?
> >>
> >>I also fixed a couple adjacent comment typos.
> >>
> >>Index: jobs.c
> >>===================================================================
> >>RCS file: /cvs/src/bin/ksh/jobs.c,v
> >>retrieving revision 1.41
> >>diff -u -p -r1.41 jobs.c
> >>--- jobs.c 18 Apr 2015 18:28:36 -0000 1.41
> >>+++ jobs.c 10 Sep 2015 00:06:59 -0000
> >>@@ -71,7 +71,7 @@ struct job {
> >>   int status; /* exit status of last process */
> >>   pid_t pgrp; /* process group of job */
> >>   pid_t ppid; /* pid of process that forked job */
> >>- INT32 age; /* number of jobs started */
> >>+ int32_t age; /* number of jobs started */
> >>   struct timeval systime; /* system time used by job */
> >>   struct timeval usrtime; /* user time used by job */
> >>   Proc *proc_list; /* process list */
> >>@@ -111,7 +111,7 @@ static Job *async_job;
> >>  static pid_t async_pid;
> >>
> >>  static int nzombie; /* # of zombies owned by this process */
> >>-INT32 njobs; /* # of jobs started */
> >>+int32_t njobs; /* # of jobs started */
> >>  static int child_max; /* CHILD_MAX */
> >>
> >>
> >>Index: sh.h
> >>===================================================================
> >>RCS file: /cvs/src/bin/ksh/sh.h,v
> >>retrieving revision 1.33
> >>diff -u -p -r1.33 sh.h
> >>--- sh.h 18 Dec 2013 13:53:12 -0000 1.33
> >>+++ sh.h 10 Sep 2015 00:06:59 -0000
> >>@@ -28,12 +28,6 @@
> >>
> >>  #include <paths.h>
> >>
> >>-/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
> >>- * by autoconf (assumes an 8 bit byte, but I'm not concerned).
> >>- * NOTE: INT32 may end up being more than 32 bits.
> >>- */
> >>-# define INT32 int
> >>-
> >>  /* end of common headers */
> >>
> >>  /* some useful #defines */
> >>@@ -52,8 +46,8 @@
> >>  #define sizeofN(type, n) (sizeof(type) * (n))
> >>  #define BIT(i) (1<<(i)) /* define bit in flag */
> >>
> >>-/* Table flag type - needs > 16 and < 32 bits */
> >>-typedef INT32 Tflag;
> >>+/* Table flag type - needs >= 16 and <= 32 bits */
> >>+typedef int32_t Tflag;
> >>
> >>  #define NUFILE 32 /* Number of user-accessible files */
> >>  #define FDBASE 10 /* First file usable by Shell */
> >>@@ -353,7 +347,7 @@ EXTERN Getopt user_opt; /* parsing stat
> >>
> >>  /* This for co-processes */
> >>
> >>-typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
> >>+typedef int32_t Coproc_id; /* something that won't (realistically) wrap */
> >>  struct coproc {
> >>   int read; /* pipe from co-process's stdout */
> >>   int readw; /* other side of read (saved temporarily) */
> >>
> >

> Index: sh.h
> ===================================================================
> RCS file: /cvs/src/bin/ksh/sh.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 sh.h
> --- sh.h 18 Dec 2013 13:53:12 -0000 1.33
> +++ sh.h 1 Sep 2015 17:23:13 -0000
> @@ -28,12 +28,6 @@
>  
>  #include <paths.h>
>  
> -/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
> - * by autoconf (assumes an 8 bit byte, but I'm not concerned).
> - * NOTE: INT32 may end up being more than 32 bits.
> - */
> -# define INT32 int
> -
>  /* end of common headers */
>  
>  /* some useful #defines */
> @@ -53,7 +47,7 @@
>  #define BIT(i) (1<<(i)) /* define bit in flag */
>  
>  /* Table flag type - needs > 16 and < 32 bits */
> -typedef INT32 Tflag;
> +typedef int Tflag;
>  
>  #define NUFILE 32 /* Number of user-accessible files */
>  #define FDBASE 10 /* First file usable by Shell */
> @@ -353,7 +347,7 @@ EXTERN Getopt user_opt; /* parsing stat
>  
>  /* This for co-processes */
>  
> -typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
> +typedef int Coproc_id; /* something that won't (realisticly) wrap */
>  struct coproc {
>   int read; /* pipe from co-process's stdout */
>   int readw; /* other side of read (saved temporarily) */
> Index: jobs.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/jobs.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 jobs.c
> --- jobs.c 18 Apr 2015 18:28:36 -0000 1.41
> +++ jobs.c 1 Sep 2015 17:23:13 -0000
> @@ -71,7 +71,7 @@ struct job {
>   int status; /* exit status of last process */
>   pid_t pgrp; /* process group of job */
>   pid_t ppid; /* pid of process that forked job */
> - INT32 age; /* number of jobs started */
> + int age; /* number of jobs started */
>   struct timeval systime; /* system time used by job */
>   struct timeval usrtime; /* user time used by job */
>   Proc *proc_list; /* process list */
> @@ -111,7 +111,7 @@ static Job *async_job;
>  static pid_t async_pid;
>  
>  static int nzombie; /* # of zombies owned by this process */
> -INT32 njobs; /* # of jobs started */
> +int njobs; /* # of jobs started */
>  static int child_max; /* CHILD_MAX */
>  
>  

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Michael McConville-2
In reply to this post by Martijn van Duren
Martijn van Duren wrote:
> I already sent this diff on September 1st.[1] Pointed out in private
> to and ok by nicm@
>
> [1]http://marc.info/?l=openbsd-tech&m=144112883814618&w=2

Ah, awkward. I hadn't seen this.

Can you clarify why you used int instead of int32_t?

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Todd C. Miller
In reply to this post by Nicholas Marriott-2
On Thu, 10 Sep 2015 12:51:37 +0100, Nicholas Marriott wrote:

> It would be more helpful to post the diff again rather than a link
> buried in another thread.
>
> Any oks for this?

OK millert@

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Martijn van Duren
In reply to this post by Michael McConville-2
On 09/10/15 15:56, Michael McConville wrote:

> Martijn van Duren wrote:
>> I already sent this diff on September 1st.[1] Pointed out in private
>> to and ok by nicm@
>>
>> [1]http://marc.info/?l=openbsd-tech&m=144112883814618&w=2
>
> Ah, awkward. I hadn't seen this.
>
> Can you clarify why you used int instead of int32_t?
>
Because that's what the original define used, so the compiled code would
be the same.

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Ted Unangst-6
In reply to this post by Nicholas Marriott-2
> - * NOTE: INT32 may end up being more than 32 bits.

>  /* Table flag type - needs > 16 and < 32 bits */
> -typedef INT32 Tflag;

awkward...

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Timo Buhrmester
In reply to this post by Michael McConville-2
> Can you clarify why you used int instead of int32_t?
Considering that
  - the comment around it said ``Find an integer type that is at least 32 bits''
  - int may be less than 32 bits wide (C99 5.2.4.2.1)
  - int32_t is not guaranteed to exist (C99 7.18.1.1p3)
The most appropriate type would be int_least32_t, no? (required to exist (C99 7.18.1.2p3))

(That said, I'm not sure if there's a port for which this could be a problem, so if not, feel free to disregard)

--
Timo Buhrmester

Reply | Threaded
Open this post in threaded view
|

Re: ksh INT32 type

Philip Guenther-2
On Fri, Sep 11, 2015 at 5:49 AM, Timo Buhrmester <[hidden email]> wrote:
>> Can you clarify why you used int instead of int32_t?
> Considering that
>   - the comment around it said ``Find an integer type that is at least 32 bits''
>   - int may be less than 32 bits wide (C99 5.2.4.2.1)

While true in a standards sense, OpenBSD will never run on an arch
where int is less than 32 bits.  Are there people porting OpenBSD's
ksh to OSes where int can be 16 bits?  If so, *they* need to step up
and argue for making to code less readable.  Uglifying the code for a
hypothetical port is not our way.

Philip Guenther