[PATCH 00/10] Add types, and some fixes

[PATCH 00/10] Add types, and some fixes

[Alejandro Colomar]

Hello Michael,

The .RS/.RE Branden and you talked about
in a previous email was useful here :-)

Cheers,

Alex

Alejandro Colomar (10):
  system_data_types.7: ffix
  system_data_types.7: Add lconv
  lconv.3: New link to system_data_types(7)
  system_data_types.7: Add float_t
  float_t.3: New link to system_data_types(7)
  system_data_types.7: Add double_t
  double_t.3: New link to system_data_types(7)
  seccomp.2: Remove unneeded cast
  seccomp.2: Use ARRAY_SIZE() macro instead of raw sizeof division
  getgrent_r.3: Declare variables with different types in different
    lines

 man2/seccomp.2           |   3 +-
 man3/double_t.3          |   1 +
 man3/float_t.3           |   1 +
 man3/getgrent_r.3        |   3 +-
 man3/lconv.3             |   1 +
 man7/system_data_types.7 | 121 ++++++++++++++++++++++++++++++++++++++-
 6 files changed, 127 insertions(+), 3 deletions(-)
 create mode 100644 man3/double_t.3
 create mode 100644 man3/float_t.3
 create mode 100644 man3/lconv.3

-- 
2.28.0

[PATCH 01/10] system_data_types.7: ffix

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man7/system_data_types.7 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index f40b1373f..095a20f6d 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -219,7 +219,7 @@ typedef struct {
 .EE
 .IP
 It is the type of the value returned by the
-.I imaxdiv (3)
+.BR imaxdiv (3)
 function.
 .IP
 Conforming to: C99 and later; POSIX.1-2001 and later.
-- 
2.28.0

[PATCH 02/10] system_data_types.7: Add lconv

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man7/system_data_types.7 | 47 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index 095a20f6d..b04457bbf 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -226,6 +226,53 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
 .IP
 See also:
 .BR imaxdiv (3)
+.\"------------------------------------- lconv ------------------------/
+.TP
+.I lconv
+.IP
+Include:
+.IR <locale.h> .
+.IP
+.EX
+struct lconv {                  /* Values in the "C" locale: */
+    char   *decimal_point;      /* "." */
+    char   *thousands_sep;      /* "" */
+    char   *grouping;           /* "" */
+    char   *mon_decimal_point;  /* "" */
+    char   *mon_thousands_sep;  /* "" */
+    char   *mon_grouping;       /* "" */
+    char   *positive_sign;      /* "" */
+    char   *negative_sign;      /* "" */
+    char   *currency_symbol;    /* "" */
+    char    frac_digits;        /* CHAR_MAX */
+    char    p_cs_precedes;      /* CHAR_MAX */
+    char    n_cs_precedes;      /* CHAR_MAX */
+    char    p_sep_by_space;     /* CHAR_MAX */
+    char    n_sep_by_space;     /* CHAR_MAX */
+    char    p_sign_posn;        /* CHAR_MAX */
+    char    n_sign_posn;        /* CHAR_MAX */
+    char   *int_curr_symbol;    /* "" */
+    char    int_frac_digits;    /* CHAR_MAX */
+    char    int_p_cs_precedes;  /* CHAR_MAX */
+    char    int_n_cs_precedes;  /* CHAR_MAX */
+    char    int_p_sep_by_space; /* CHAR_MAX */
+    char    int_n_sep_by_space; /* CHAR_MAX */
+    char    int_p_sign_posn;    /* CHAR_MAX */
+    char    int_n_sign_posn;    /* CHAR_MAX */
+};
+.EE
+.IP
+Contains members related to the formatting of numeric values.
+In the "C" locale, its members have the values
+shown in the comments above.
+.IP
+Conforming to: C11 and later; POSIX.1-2001 and later.
+.IP
+See also:
+.BR setlocale (3),
+.BR localeconv (3),
+.BR charsets (5),
+.BR locale (7)
 .\"------------------------------------- ldiv_t -----------------------/
 .TP
 .I ldiv_t
-- 
2.28.0

[PATCH 03/10] lconv.3: New link to system_data_types(7)

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man3/lconv.3 | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 man3/lconv.3

diff --git a/man3/lconv.3 b/man3/lconv.3
new file mode 100644
index 000000000..db50c0f09
--- /dev/null
+++ b/man3/lconv.3
@@ -0,0 +1 @@
+.so man7/system_data_types.7
-- 
2.28.0

[PATCH 04/10] system_data_types.7: Add float_t

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man7/system_data_types.7 | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index b04457bbf..238b9593b 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -147,6 +147,42 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
 .IP
 See also:
 .BR fenv (3)
+.\"------------------------------------- float_t ----------------------/
+.TP
+.I float_t
+.IP
+Include:
+.IR <math.h> .
+.IP
+The implementation's most efficient floating type at least as wide as
+.IR float .
+Its type depends on the value of the macro
+.BR FLT_EVAL_METHOD :
+.RS
+.IP *
+0;
+.I float_t
+is
+.IR float .
+.IP *
+1;
+.I float_t
+is
+.IR double .
+.IP *
+2;
+.I float_t
+is
+.IR "long double" .
+.IP *
+Other implementation-defined values.
+.RE
+.IP
+Conforming to: C99 and later; POSIX.1-2001 and later.
+.IP
+See also the
+.I double_t
+type in this page.
 .\"------------------------------------- gid_t ------------------------/
 .TP
 .I gid_t
-- 
2.28.0

[PATCH 05/10] float_t.3: New link to system_data_types(7)

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man3/float_t.3 | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 man3/float_t.3

diff --git a/man3/float_t.3 b/man3/float_t.3
new file mode 100644
index 000000000..db50c0f09
--- /dev/null
+++ b/man3/float_t.3
@@ -0,0 +1 @@
+.so man7/system_data_types.7
-- 
2.28.0

[PATCH 06/10] system_data_types.7: Add double_t

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man7/system_data_types.7 | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index 238b9593b..43e88942a 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -117,6 +117,42 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
 .IP
 See also:
 .BR div (3)
+.\"------------------------------------- double_t ---------------------/
+.TP
+.I double_t
+.IP
+Include:
+.IR <math.h> .
+.IP
+The implementation's most efficient floating type at least as wide as
+.IR double .
+Its type depends on the value of the macro
+.BR FLT_EVAL_METHOD :
+.RS
+.IP *
+0;
+.I double_t
+is
+.IR double .
+.IP *
+1;
+.I double_t
+is
+.IR double .
+.IP *
+2;
+.I double_t
+is
+.IR "long double" .
+.IP *
+Other implementation-defined values.
+.RE
+.IP
+Conforming to: C99 and later; POSIX.1-2001 and later.
+.IP
+See also the
+.I float_t
+type in this page.
 .\"------------------------------------- fenv_t -----------------------/
 .TP
 .I fenv_t
-- 
2.28.0

[PATCH 07/10] double_t.3: New link to system_data_types(7)

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man3/double_t.3 | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 man3/double_t.3

diff --git a/man3/double_t.3 b/man3/double_t.3
new file mode 100644
index 000000000..db50c0f09
--- /dev/null
+++ b/man3/double_t.3
@@ -0,0 +1 @@
+.so man7/system_data_types.7
-- 
2.28.0

[PATCH 08/10] seccomp.2: Remove unneeded cast

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man2/seccomp.2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man2/seccomp.2 b/man2/seccomp.2
index 58033da1c..d6b856c32 100644
--- a/man2/seccomp.2
+++ b/man2/seccomp.2
@@ -1101,7 +1101,7 @@ install_filter(int syscall_nr, int t_arch, int f_errno)
     };
 
     struct sock_fprog prog = {
-        .len = (unsigned short) (sizeof(filter) / sizeof(filter[0])),
+        .len = sizeof(filter) / sizeof(filter[0]),
         .filter = filter,
     };
 
-- 
2.28.0

[PATCH 09/10] seccomp.2: Use ARRAY_SIZE() macro instead of raw sizeof division

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man2/seccomp.2 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/man2/seccomp.2 b/man2/seccomp.2
index d6b856c32..82eb04dc6 100644
--- a/man2/seccomp.2
+++ b/man2/seccomp.2
@@ -1051,6 +1051,7 @@ cecilia
 #include <sys/prctl.h>
 
 #define X32_SYSCALL_BIT 0x40000000
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
 static int
 install_filter(int syscall_nr, int t_arch, int f_errno)
@@ -1101,7 +1102,7 @@ install_filter(int syscall_nr, int t_arch, int f_errno)
     };
 
     struct sock_fprog prog = {
-        .len = sizeof(filter) / sizeof(filter[0]),
+        .len = ARRAY_SIZE(filter),
         .filter = filter,
     };
 
-- 
2.28.0

[PATCH 10/10] getgrent_r.3: Declare variables with different types in different lines

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man3/getgrent_r.3 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
index 8170cd417..64d7f3ac1 100644
--- a/man3/getgrent_r.3
+++ b/man3/getgrent_r.3
@@ -181,7 +181,8 @@ in the stream with all other threads.
 int
 main(void)
 {
-    struct group grp, *grpp;
+    struct group grp;
+    struct group *grpp;
     char buf[BUFLEN];
     int i;
 
-- 
2.28.0

Re: [PATCH 01/10] system_data_types.7: ffix

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Thanks, Alex. Patch applied.

Cheers,

Michael

> ---
>  man7/system_data_types.7 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
> index f40b1373f..095a20f6d 100644
> --- a/man7/system_data_types.7
> +++ b/man7/system_data_types.7
> @@ -219,7 +219,7 @@ typedef struct {
>  .EE
>  .IP
>  It is the type of the value returned by the
> -.I imaxdiv (3)
> +.BR imaxdiv (3)
>  function.
>  .IP
>  Conforming to: C99 and later; POSIX.1-2001 and later.
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 10/10] getgrent_r.3: Declare variables with different types in different lines

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Thanks, Alex. Patch applied.

Cheers,

Michael

> ---
>  man3/getgrent_r.3 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3
> index 8170cd417..64d7f3ac1 100644
> --- a/man3/getgrent_r.3
> +++ b/man3/getgrent_r.3
> @@ -181,7 +181,8 @@ in the stream with all other threads.
>  int
>  main(void)
>  {
> -    struct group grp, *grpp;
> +    struct group grp;
> +    struct group *grpp;
>      char buf[BUFLEN];
>      int i;
>  
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 02/10] system_data_types.7: Add lconv

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Patch applied. Thanks, Alex.

Cheers,

Michael


> ---
>  man7/system_data_types.7 | 47 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
> index 095a20f6d..b04457bbf 100644
> --- a/man7/system_data_types.7
> +++ b/man7/system_data_types.7
> @@ -226,6 +226,53 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
>  .IP
>  See also:
>  .BR imaxdiv (3)
> +.\"------------------------------------- lconv ------------------------/
> +.TP
> +.I lconv
> +.IP
> +Include:
> +.IR <locale.h> .
> +.IP
> +.EX
> +struct lconv {                  /* Values in the "C" locale: */
> +    char   *decimal_point;      /* "." */
> +    char   *thousands_sep;      /* "" */
> +    char   *grouping;           /* "" */
> +    char   *mon_decimal_point;  /* "" */
> +    char   *mon_thousands_sep;  /* "" */
> +    char   *mon_grouping;       /* "" */
> +    char   *positive_sign;      /* "" */
> +    char   *negative_sign;      /* "" */
> +    char   *currency_symbol;    /* "" */
> +    char    frac_digits;        /* CHAR_MAX */
> +    char    p_cs_precedes;      /* CHAR_MAX */
> +    char    n_cs_precedes;      /* CHAR_MAX */
> +    char    p_sep_by_space;     /* CHAR_MAX */
> +    char    n_sep_by_space;     /* CHAR_MAX */
> +    char    p_sign_posn;        /* CHAR_MAX */
> +    char    n_sign_posn;        /* CHAR_MAX */
> +    char   *int_curr_symbol;    /* "" */
> +    char    int_frac_digits;    /* CHAR_MAX */
> +    char    int_p_cs_precedes;  /* CHAR_MAX */
> +    char    int_n_cs_precedes;  /* CHAR_MAX */
> +    char    int_p_sep_by_space; /* CHAR_MAX */
> +    char    int_n_sep_by_space; /* CHAR_MAX */
> +    char    int_p_sign_posn;    /* CHAR_MAX */
> +    char    int_n_sign_posn;    /* CHAR_MAX */
> +};
> +.EE
> +.IP
> +Contains members related to the formatting of numeric values.
> +In the "C" locale, its members have the values
> +shown in the comments above.
> +.IP
> +Conforming to: C11 and later; POSIX.1-2001 and later.
> +.IP
> +See also:
> +.BR setlocale (3),
> +.BR localeconv (3),
> +.BR charsets (5),
> +.BR locale (7)
>  .\"------------------------------------- ldiv_t -----------------------/
>  .TP
>  .I ldiv_t
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 03/10] lconv.3: New link to system_data_types(7)

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Thanks, Alex. Patch applied.

Cheers,

Michael


> ---
>  man3/lconv.3 | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 man3/lconv.3
> 
> diff --git a/man3/lconv.3 b/man3/lconv.3
> new file mode 100644
> index 000000000..db50c0f09
> --- /dev/null
> +++ b/man3/lconv.3
> @@ -0,0 +1 @@
> +.so man7/system_data_types.7
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 04/10] system_data_types.7: Add float_t

[Michael Kerrisk (man-pages)]

Hi Alex,

A few comments here that also apply for the double_t patch. 
Could you revise please?

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
>  man7/system_data_types.7 | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
> index b04457bbf..238b9593b 100644
> --- a/man7/system_data_types.7
> +++ b/man7/system_data_types.7
> @@ -147,6 +147,42 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
>  .IP
>  See also:
>  .BR fenv (3)
> +.\"------------------------------------- float_t ----------------------/
> +.TP
> +.I float_t
> +.IP
> +Include:
> +.IR <math.h> .
> +.IP
> +The implementation's most efficient floating type at least as wide as
> +.IR float .

The C standard is rather terse here. Perhaps we could make the 
reader's life a little easier. How about something like:

[[
This a type that is intended to be the implementation's
most efficient floating type that is at least as wide as
.IR float .
]]

> +Its type depends on the value of the macro
> +.BR FLT_EVAL_METHOD :
> +.RS
> +.IP *
> +0;
> +.I float_t
> +is
> +.IR float .
> +.IP *
> +1;
> +.I float_t
> +is
> +.IR double .
> +.IP *
> +2;
> +.I float_t
> +is
> +.IR "long double" .
> +.IP *
> +Other implementation-defined values.
> +.RE

I think we can format this better as a hanging list.
Something like this:

[[
.TP
0
.I float_t
is
.IR float .
.TP
1
.I float_t
is
.IR double .
.TP
2
.I float_t
is
.IR "long double" .
.IP
For other values of
.BR FLT_EVAL_METHOD ,
the type of
.I float_t
is implementation-defined.
]]

> +.IP
> +Conforming to: C99 and later; POSIX.1-2001 and later.
> +.IP
> +See also the
> +.I double_t
> +type in this page.
>  .\"------------------------------------- gid_t ------------------------/
>  .TP
>  .I gid_t

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 04/10] system_data_types.7: Add float_t

[Alejandro Colomar]

Hi Michael,

On 2020-09-25 10:13, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
> 
> A few comments here that also apply for the double_t patch.
> Could you revise please?
> 
> On 9/25/20 9:31 AM, Alejandro Colomar wrote:
>> +The implementation's most efficient floating type at least as wide as
>> +.IR float .
> 
> The C standard is rather terse here. Perhaps we could make the
> reader's life a little easier. How about something like:
> 
> [[
> This a type that is intended to be the implementation's
> most efficient floating type that is at least as wide as
> .IR float .
> ]]

I removed the "intended" part to simplify it a bit. Do you prefer to 
keep it?

> 
>> +Its type depends on the value of the macro
>> +.BR FLT_EVAL_METHOD :
>> +.RS
>> +.IP *
>> +0;
>> +.I float_t
>> +is
>> +.IR float .
>> +.IP *
>> +1;
>> +.I float_t
>> +is
>> +.IR double .
>> +.IP *
>> +2;
>> +.I float_t
>> +is
>> +.IR "long double" .
>> +.IP *
>> +Other implementation-defined values.
>> +.RE
> 
> I think we can format this better as a hanging list.
> Something like this:
> 
> [[
> .TP
> 0
> .I float_t
> is
> .IR float .
> .TP
> 1
> .I float_t
> is
> .IR double .
> .TP
> 2
> .I float_t
> is
> .IR "long double" .
> .IP
> For other values of
> .BR FLT_EVAL_METHOD ,
> the type of
> .I float_t
> is implementation-defined.
> ]
Yes, I wasn't convinced by my formatting.  Thanks!

I'll fix this patch, and the srcfix that depends on this too.

BTW, I'll also add a note that FLT_EVAL_METHOD is defined in <float.h>
Would you add that to "Notes", or as part of the description?

> 
>> +.IP
>> +Conforming to: C99 and later; POSIX.1-2001 and later.
>> +.IP
>> +See also the
>> +.I double_t
>> +type in this page.
>>   .\"------------------------------------- gid_t ------------------------/
>>   .TP
>>   .I gid_t
> 
> Thanks,
> 
> Michael
> 
> 
Thanks,

Alex

Re: [PATCH 08/10] seccomp.2: Remove unneeded cast

[Michael Kerrisk (man-pages)]

Hi Alex,

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
>  man2/seccomp.2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man2/seccomp.2 b/man2/seccomp.2
> index 58033da1c..d6b856c32 100644
> --- a/man2/seccomp.2
> +++ b/man2/seccomp.2
> @@ -1101,7 +1101,7 @@ install_filter(int syscall_nr, int t_arch, int f_errno)
>      };
>  
>      struct sock_fprog prog = {
> -        .len = (unsigned short) (sizeof(filter) / sizeof(filter[0])),
> +        .len = sizeof(filter) / sizeof(filter[0]),
>          .filter = filter,
>      };

I have a small doubt about this change. With the change,
there are no compilation warnings.

But, if we change the code to something slightly different:

[[
    size_t x = (sizeof(filter) / sizeof(filter[0]));
    struct sock_fprog prog = {
        .len = x,
        .filter = filter,
    };
]]

The "cc -Wconversion" gives us the following warning:

    warning: conversion from ‘size_t’ {aka ‘long unsigned int’}
    to ‘short unsigned int’ may change value

Presumably we don't get a warning for an assignment of the form

    .len = (sizeof(filter) / sizeof(filter[0]))

because the compiler is smart enough to work out that the
value of the constant expression is within the range of
"unsigned short".

Your thoughts?

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 08/10] seccomp.2: Remove unneeded cast

[Alejandro Colomar]

On 2020-09-25 10:34, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
> 
> On 9/25/20 9:31 AM, Alejandro Colomar wrote:
>> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
>> ---
>>   man2/seccomp.2 | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/man2/seccomp.2 b/man2/seccomp.2
>> index 58033da1c..d6b856c32 100644
>> --- a/man2/seccomp.2
>> +++ b/man2/seccomp.2
>> @@ -1101,7 +1101,7 @@ install_filter(int syscall_nr, int t_arch, int f_errno)
>>       };
>>   
>>       struct sock_fprog prog = {
>> -        .len = (unsigned short) (sizeof(filter) / sizeof(filter[0])),
>> +        .len = sizeof(filter) / sizeof(filter[0]),
>>           .filter = filter,
>>       };
> 
> I have a small doubt about this change. With the change,
> there are no compilation warnings.
> 
> But, if we change the code to something slightly different:
> 
> [[
>      size_t x = (sizeof(filter) / sizeof(filter[0]));
>      struct sock_fprog prog = {
>          .len = x,
>          .filter = filter,
>      };
> ]]
> 
> The "cc -Wconversion" gives us the following warning:
> 
>      warning: conversion from ‘size_t’ {aka ‘long unsigned int’}
>      to ‘short unsigned int’ may change value
> 
> Presumably we don't get a warning for an assignment of the form
> 
>      .len = (sizeof(filter) / sizeof(filter[0]))
> 
> because the compiler is smart enough to work out that the
> value of the constant expression is within the range of
> "unsigned short".
> 
> Your thoughts?

Hi Michael,

I'd say that the cast doesn't fix any problems at all.  It silences a 
valid warning, and I'd use a pragma for that (to be more explicit about 
the intention of silencing a warning) if I do want -Wconversion enabled 
(which usually I don't want, because it's too noisy) and I'm sure that 
this won't overflow.  I'd limit the use casts to only when I *really* 
need to.

I guess that if you enable -O3, the warning will vanish again because 
the compiler will optimize away 'x' (but I didn't test).

Thanks,

Alex

> 
> Thanks,
> 
> Michael
> 
> 

Re: [PATCH 08/10] seccomp.2: Remove unneeded cast

[Michael Kerrisk (man-pages)]

On 9/25/20 10:42 AM, Alejandro Colomar wrote:
> 
> 
> On 2020-09-25 10:34, Michael Kerrisk (man-pages) wrote:
>> Hi Alex,
>>
>> On 9/25/20 9:31 AM, Alejandro Colomar wrote:
>>> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
>>> ---
>>>   man2/seccomp.2 | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/man2/seccomp.2 b/man2/seccomp.2
>>> index 58033da1c..d6b856c32 100644
>>> --- a/man2/seccomp.2
>>> +++ b/man2/seccomp.2
>>> @@ -1101,7 +1101,7 @@ install_filter(int syscall_nr, int t_arch, int f_errno)
>>>       };
>>>   
>>>       struct sock_fprog prog = {
>>> -        .len = (unsigned short) (sizeof(filter) / sizeof(filter[0])),
>>> +        .len = sizeof(filter) / sizeof(filter[0]),
>>>           .filter = filter,
>>>       };
>>
>> I have a small doubt about this change. With the change,
>> there are no compilation warnings.
>>
>> But, if we change the code to something slightly different:
>>
>> [[
>>      size_t x = (sizeof(filter) / sizeof(filter[0]));
>>      struct sock_fprog prog = {
>>          .len = x,
>>          .filter = filter,
>>      };
>> ]]
>>
>> The "cc -Wconversion" gives us the following warning:
>>
>>      warning: conversion from ‘size_t’ {aka ‘long unsigned int’}
>>      to ‘short unsigned int’ may change value
>>
>> Presumably we don't get a warning for an assignment of the form
>>
>>      .len = (sizeof(filter) / sizeof(filter[0]))
>>
>> because the compiler is smart enough to work out that the
>> value of the constant expression is within the range of
>> "unsigned short".
>>
>> Your thoughts?
> 
> Hi Michael,
> 
> I'd say that the cast doesn't fix any problems at all.  It silences a 
> valid warning, and I'd use a pragma for that (to be more explicit about 
> the intention of silencing a warning) if I do want -Wconversion enabled 
> (which usually I don't want, because it's too noisy) and I'm sure that 
> this won't overflow.  I'd limit the use casts to only when I *really* 
> need to.
> 
> I guess that if you enable -O3, the warning will vanish again because 
> the compiler will optimize away 'x' (but I didn't test).

Fair enough! I've applied the patch. Thanks!

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 09/10] seccomp.2: Use ARRAY_SIZE() macro instead of raw sizeof division

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Thanks, Alex. Patch applied.

Cheers,

Michael

> ---
>  man2/seccomp.2 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/man2/seccomp.2 b/man2/seccomp.2
> index d6b856c32..82eb04dc6 100644
> --- a/man2/seccomp.2
> +++ b/man2/seccomp.2
> @@ -1051,6 +1051,7 @@ cecilia
>  #include <sys/prctl.h>
>  
>  #define X32_SYSCALL_BIT 0x40000000
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>  
>  static int
>  install_filter(int syscall_nr, int t_arch, int f_errno)
> @@ -1101,7 +1102,7 @@ install_filter(int syscall_nr, int t_arch, int f_errno)
>      };
>  
>      struct sock_fprog prog = {
> -        .len = sizeof(filter) / sizeof(filter[0]),
> +        .len = ARRAY_SIZE(filter),
>          .filter = filter,
>      };
>  
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 04/10] system_data_types.7: Add float_t

[Michael Kerrisk (man-pages)]

Hi Alex,

On 9/25/20 10:22 AM, Alejandro Colomar wrote:
> Hi Michael,
> 
> On 2020-09-25 10:13, Michael Kerrisk (man-pages) wrote:
>> Hi Alex,
>>
>> A few comments here that also apply for the double_t patch.
>> Could you revise please?
>>
>> On 9/25/20 9:31 AM, Alejandro Colomar wrote:
>>> +The implementation's most efficient floating type at least as wide as
>>> +.IR float .
>>
>> The C standard is rather terse here. Perhaps we could make the
>> reader's life a little easier. How about something like:
>>
>> [[
>> This a type that is intended to be the implementation's
>> most efficient floating type that is at least as wide as
>> .IR float .
>> ]]
> 
> I removed the "intended" part to simplify it a bit. Do you prefer to 
> keep it?

Well, as long as you are going to lift the detail about "most
efficient" from the C standard, I'd be inclined to keep 
the word "intended" from the standard as well. But I do not feel
strongly about it.

>>> +Its type depends on the value of the macro
>>> +.BR FLT_EVAL_METHOD :
>>> +.RS
>>> +.IP *
>>> +0;
>>> +.I float_t
>>> +is
>>> +.IR float .
>>> +.IP *
>>> +1;
>>> +.I float_t
>>> +is
>>> +.IR double .
>>> +.IP *
>>> +2;
>>> +.I float_t
>>> +is
>>> +.IR "long double" .
>>> +.IP *
>>> +Other implementation-defined values.
>>> +.RE
>>
>> I think we can format this better as a hanging list.
>> Something like this:
>>
>> [[
>> .TP
>> 0
>> .I float_t
>> is
>> .IR float .
>> .TP
>> 1
>> .I float_t
>> is
>> .IR double .
>> .TP
>> 2
>> .I float_t
>> is
>> .IR "long double" .
>> .IP
>> For other values of
>> .BR FLT_EVAL_METHOD ,
>> the type of
>> .I float_t
>> is implementation-defined.
>> ]
> Yes, I wasn't convinced by my formatting.  Thanks!
> 
> I'll fix this patch, and the srcfix that depends on this too.

Okay.

> BTW, I'll also add a note that FLT_EVAL_METHOD is defined in <float.h>
> Would you add that to "Notes", or as part of the description?

As part of the description I think. (And I don't mind if it's 
repeated in the double_t description.)

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

[PATCH v2 04/10] system_data_types.7: Add float_t

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---

Hi Michael,

On 2020-09-25 11:30, Michael Kerrisk (man-pages) wrote:
>> I removed the "intended" part to simplify it a bit. Do you prefer to
>> keep it?
> Well, as long as you are going to lift the detail about "most
> efficient" from the C standard, I'd be inclined to keep
> the word "intended" from the standard as well. But I do not feel
> strongly about it.
> 

Fair enough.


On 2020-09-25 11:30, Michael Kerrisk (man-pages) wrote:
>> BTW, I'll also add a note that FLT_EVAL_METHOD is defined in <float.h>
>> Would you add that to "Notes", or as part of the description?
> As part of the description I think. (And I don't mind if it's
> repeated in the double_t description.)

I think there was a misunderstanding; maybe I should have been clearer:

I meant to write about <float.h> in the "Notes" of the type,
and not in the "NOTES" of the entire page.

However, I decided to do it in the description of the type as you said,
because it was a bit simpler.

The other version would have resulted in this code, just for you to see:

[[
.TP
.I double_t
.RS
.PP
Include:
.IR <math.h> .
.PP
The implementation's most efficient floating type at least as wide as
.IR double .
Its type depends on the value of the macro
.BR FLT_EVAL_METHOD :
.TP
0
.I double_t
is
.IR double .
.TP
1
.I double_t
is
.IR double .
.TP
2
.I double_t
is
.IR "long double" .
.IP
For other values of
.BR FLT_EVAL_METHOD ,
the type of
.I double_t
is implementation-defined.
.PP
Conforming to: C99 and later; POSIX.1-2001 and later.
.PP
Notes: The macro
.B FLT_EVAL_METHOD
is defined in
.IR <float.h> .
.PP
See also the
.I float_t
type in this page.
.RE
]]


Cheers,

Alex

 man7/system_data_types.7 | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index b04457bbf..aa5ab95e9 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -147,6 +147,48 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
 .IP
 See also:
 .BR fenv (3)
+.\"------------------------------------- float_t ----------------------/
+.TP
+.I float_t
+.IP
+Include:
+.IR <math.h> .
+.IP
+The implementation's most efficient floating type at least as wide as
+.IR float .
+Its type depends on the value of the macro
+.BR FLT_EVAL_METHOD ,
+defined in
+.IR <float.h> :
+.RS
+.TP
+0
+.I float_t
+is
+.IR float .
+.TP
+1
+.I float_t
+is
+.IR double .
+.TP
+2
+.I float_t
+is
+.IR "long double" .
+.IP
+For other values of
+.BR FLT_EVAL_METHOD ,
+the type of
+.I float_t
+is implementation-defined.
+.RE
+.IP
+Conforming to: C99 and later; POSIX.1-2001 and later.
+.IP
+See also the
+.I double_t
+type in this page.
 .\"------------------------------------- gid_t ------------------------/
 .TP
 .I gid_t
-- 
2.28.0

[PATCH v2 06/10] system_data_types.7: Add double_t

[Alejandro Colomar]

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man7/system_data_types.7 | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index aa5ab95e9..7730815d4 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -117,6 +117,48 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
 .IP
 See also:
 .BR div (3)
+.\"------------------------------------- double_t ---------------------/
+.TP
+.I double_t
+.IP
+Include:
+.IR <math.h> .
+.IP
+The implementation's most efficient floating type at least as wide as
+.IR double .
+Its type depends on the value of the macro
+.BR FLT_EVAL_METHOD ,
+defined in
+.IR <float.h> :
+.RS
+.TP
+0
+.I double_t
+is
+.IR double .
+.TP
+1
+.I double_t
+is
+.IR double .
+.TP
+2
+.I double_t
+is
+.IR "long double" .
+.IP
+For other values of
+.BR FLT_EVAL_METHOD ,
+the type of
+.I double_t
+is implementation-defined.
+.RE
+.IP
+Conforming to: C99 and later; POSIX.1-2001 and later.
+.IP
+See also the
+.I float_t
+type in this page.
 .\"------------------------------------- fenv_t -----------------------/
 .TP
 .I fenv_t
-- 
2.28.0

Re: [PATCH v2 04/10] system_data_types.7: Add float_t

[Michael Kerrisk (man-pages)]

Hi Alex,

On 9/25/20 1:19 PM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
> 
> Hi Michael,
> 
> On 2020-09-25 11:30, Michael Kerrisk (man-pages) wrote:
>>> I removed the "intended" part to simplify it a bit. Do you prefer to
>>> keep it?
>> Well, as long as you are going to lift the detail about "most
>> efficient" from the C standard, I'd be inclined to keep
>> the word "intended" from the standard as well. But I do not feel
>> strongly about it.
>>
> 
> Fair enough.
> 
> 
> On 2020-09-25 11:30, Michael Kerrisk (man-pages) wrote:
>>> BTW, I'll also add a note that FLT_EVAL_METHOD is defined in <float.h>
>>> Would you add that to "Notes", or as part of the description?
>> As part of the description I think. (And I don't mind if it's
>> repeated in the double_t description.)
> 
> I think there was a misunderstanding; maybe I should have been clearer:
> 
> I meant to write about <float.h> in the "Notes" of the type,
> and not in the "NOTES" of the entire page.

Okay -- got it now.

> 
> However, I decided to do it in the description of the type as you said,
> because it was a bit simpler.
> 
> The other version would have resulted in this code, just for you to see:

I think the path you took was better. I've applied the patch and 
done some very light editing.

Thanks,

Michael


> 
> [[
> .TP
> .I double_t
> .RS
> .PP
> Include:
> .IR <math.h> .
> .PP
> The implementation's most efficient floating type at least as wide as
> .IR double .
> Its type depends on the value of the macro
> .BR FLT_EVAL_METHOD :
> .TP
> 0
> .I double_t
> is
> .IR double .
> .TP
> 1
> .I double_t
> is
> .IR double .
> .TP
> 2
> .I double_t
> is
> .IR "long double" .
> .IP
> For other values of
> .BR FLT_EVAL_METHOD ,
> the type of
> .I double_t
> is implementation-defined.
> .PP
> Conforming to: C99 and later; POSIX.1-2001 and later.
> .PP
> Notes: The macro
> .B FLT_EVAL_METHOD
> is defined in
> .IR <float.h> .
> .PP
> See also the
> .I float_t
> type in this page.
> .RE
> ]]
> 
> 
> Cheers,
> 
> Alex
> 
>  man7/system_data_types.7 | 42 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
> index b04457bbf..aa5ab95e9 100644
> --- a/man7/system_data_types.7
> +++ b/man7/system_data_types.7
> @@ -147,6 +147,48 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
>  .IP
>  See also:
>  .BR fenv (3)
> +.\"------------------------------------- float_t ----------------------/
> +.TP
> +.I float_t
> +.IP
> +Include:
> +.IR <math.h> .
> +.IP
> +The implementation's most efficient floating type at least as wide as
> +.IR float .
> +Its type depends on the value of the macro
> +.BR FLT_EVAL_METHOD ,
> +defined in
> +.IR <float.h> :
> +.RS
> +.TP
> +0
> +.I float_t
> +is
> +.IR float .
> +.TP
> +1
> +.I float_t
> +is
> +.IR double .
> +.TP
> +2
> +.I float_t
> +is
> +.IR "long double" .
> +.IP
> +For other values of
> +.BR FLT_EVAL_METHOD ,
> +the type of
> +.I float_t
> +is implementation-defined.
> +.RE
> +.IP
> +Conforming to: C99 and later; POSIX.1-2001 and later.
> +.IP
> +See also the
> +.I double_t
> +type in this page.
>  .\"------------------------------------- gid_t ------------------------/
>  .TP
>  .I gid_t
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 05/10] float_t.3: New link to system_data_types(7)

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Applied. Thanks, Alex.

Cheers,

Michael


> ---
>  man3/float_t.3 | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 man3/float_t.3
> 
> diff --git a/man3/float_t.3 b/man3/float_t.3
> new file mode 100644
> index 000000000..db50c0f09
> --- /dev/null
> +++ b/man3/float_t.3
> @@ -0,0 +1 @@
> +.so man7/system_data_types.7
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 06/10] system_data_types.7: Add double_t

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>


Applied. Thanks, Alex.

Cheers,

Michael

> ---
>  man7/system_data_types.7 | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
> index 238b9593b..43e88942a 100644
> --- a/man7/system_data_types.7
> +++ b/man7/system_data_types.7
> @@ -117,6 +117,42 @@ Conforming to: C99 and later; POSIX.1-2001 and later.
>  .IP
>  See also:
>  .BR div (3)
> +.\"------------------------------------- double_t ---------------------/
> +.TP
> +.I double_t
> +.IP
> +Include:
> +.IR <math.h> .
> +.IP
> +The implementation's most efficient floating type at least as wide as
> +.IR double .
> +Its type depends on the value of the macro
> +.BR FLT_EVAL_METHOD :
> +.RS
> +.IP *
> +0;
> +.I double_t
> +is
> +.IR double .
> +.IP *
> +1;
> +.I double_t
> +is
> +.IR double .
> +.IP *
> +2;
> +.I double_t
> +is
> +.IR "long double" .
> +.IP *
> +Other implementation-defined values.
> +.RE
> +.IP
> +Conforming to: C99 and later; POSIX.1-2001 and later.
> +.IP
> +See also the
> +.I float_t
> +type in this page.
>  .\"------------------------------------- fenv_t -----------------------/
>  .TP
>  .I fenv_t
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 07/10] double_t.3: New link to system_data_types(7)

[Michael Kerrisk (man-pages)]

On 9/25/20 9:31 AM, Alejandro Colomar wrote:
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>


Applied. Thanks, Alex.

Cheers,

Michael


> ---
>  man3/double_t.3 | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 man3/double_t.3
> 
> diff --git a/man3/double_t.3 b/man3/double_t.3
> new file mode 100644
> index 000000000..db50c0f09
> --- /dev/null
> +++ b/man3/double_t.3
> @@ -0,0 +1 @@
> +.so man7/system_data_types.7
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/