Re: [PATCH v8] C, ObjC: Add -Wunterminated-string-initialization
Ping. I see that GCC-14 has been released recently. This is a gentle ping to see if this is a better time for this patch. Have a lovely day! Alex signature.asc Description: PGP signature
[PATCH v8] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Since Wc++-compat now includes this warning, the test has to be modified
to expect the text of the new warning too, in .
gcc/c-family/ChangeLog:
* c.opt: Add -Wunterminated-string-initialization.
gcc/c/ChangeLog:
* c-typeck.cc (digest_init): Separate warnings about character
arrays being initialized as unterminated character sequences
with string literals, from -Wc++-compat, into a new warning,
-Wunterminated-string-initialization.
gcc/ChangeLog:
* doc/invoke.texi: Document the new
-Wunterminated-string-initialization.
gcc/testsuite/ChangeLog:
* gcc.dg/Wcxx-compat-14.c: Adapt the test to match the new text
of the warning, which doesn't say anything about C++ anymore.
* gcc.dg/Wunterminated-string-initialization.c: New test.
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
[Sandra: The documentation parts of the patch are OK.]
Reviewed-by: Sandra Loosemore
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Cc: Mike Stump
Cc: Joseph Myers
Signed-off-by: Alejandro Colomar
---
Hi!
I've added a changelog to the commit message, as requested by Sandra,
and noted her review.
Have a lovely day!
Alex
Range-diff against v7:
1: c0f3ffcca7a ! 1: 06236d0aa05 C, ObjC: Add
-Wunterminated-string-initialization
@@ Commit message
Since Wc++-compat now includes this warning, the test has to be
modified
to expect the text of the new warning too, in
.
+gcc/c-family/ChangeLog:
+
+* c.opt: Add -Wunterminated-string-initialization.
+
+gcc/c/ChangeLog:
+
+* c-typeck.cc (digest_init): Separate warnings about character
+ arrays being initialized as unterminated character sequences
+ with string literals, from -Wc++-compat, into a new warning,
+ -Wunterminated-string-initialization.
+
+gcc/ChangeLog:
+
+* doc/invoke.texi: Document the new
+ -Wunterminated-string-initialization.
+
+gcc/testsuite/ChangeLog:
+
+* gcc.dg/Wcxx-compat-14.c: Adapt the test to match the new text
+ of the warning, which doesn't say anything about C++ anymore.
+* gcc.dg/Wunterminated-string-initialization.c: New test.
+
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
+[Sandra: The documentation parts of the patch are OK.]
+Reviewed-by: Sandra Loosemore
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
@@ Commit message
Cc: David Malcolm
Cc: Mike Stump
Cc: Joseph Myers
-Cc: Sandra Loosemore
Signed-off-by: Alejandro Colomar
## gcc/c-family/c.opt ##
gcc/c-family/c.opt| 4
gcc/c/c-typeck.cc | 6 +++---
gcc/doc/invoke.texi
[PATCH v7] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Since Wc++-compat now includes this warning, the test has to be modified
to expect the text of the new warning too, in .
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Cc: Mike Stump
Cc: Joseph Myers
Cc: Sandra Loosemore
Signed-off-by: Alejandro Colomar
---
Range-diff against v6:
1: e8fd975bde7 ! 1: c0f3ffcca7a C, ObjC: Add
-Wunterminated-string-initialization
@@ gcc/doc/invoke.texi: arithmetic that may yield out of bounds values.
This warnin
+@opindex Wunterminated-string-initialization
+@opindex Wno-unterminated-string-initialization
-+@item -Wunterminated-string-initialization
++@item -Wunterminated-string-initialization @r{(C and Objective-C only)}
+Warn about character arrays
+initialized as unterminated character sequences
+with a string literal.
@@ gcc/doc/invoke.texi: arithmetic that may yield out of bounds values.
This warnin
+char arr[3] = "foo";
+@end smallexample
+
-+@option{-Wunterminated-string-initialization} is enabled by
@option{-Wextra}.
++This warning is enabled by @option{-Wextra} and @option{-Wc++-compat}.
++In C++, such initializations are an error.
+
@opindex Warray-compare
@opindex Wno-array-compare
gcc/c-family/c.opt| 4
gcc/c/c-typeck.cc | 6 +++---
gcc/doc/invoke.texi | 20 ++-
gcc/testsuite/gcc.dg/Wcxx-compat-14.c | 2 +-
.../Wunterminated-string-initialization.c | 6 ++
5 files changed, 33 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44b9c862c14..3837021747b 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
C ObjC Var(warn_unsuffixed_float_constants) Warning
Warn about unsuffixed float constants.
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences
with a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e55e887da14..7df9de819ed 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree init,
tree origtype,
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
- else if (warn_cxx_compat
+ else if (warn_unterminated_string_initialization
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wc___compat,
+ warning_at (init_loc, OPT_Wunterminated_string_initialization,
("initializer-string for array of %qT "
-"is too long
Re: [PATCH v6] C, ObjC: Add -Wunterminated-string-initialization
On Tue, Mar 05, 2024 at 09:20:42PM +0100, Alejandro Colomar wrote:
> Hi!
>
> v6:
> - Small wording fix in c.opt
> - Document the option in invoke.texi
>
> I tried again, but didn't find much alphabetic order in there, so put
> it where Mike suggested, after -Warray-bounds=n.
>
> Have a lovely night!
> Alex
>
[...]
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 146b40414b0..f81df4de934 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -410,7 +410,9 @@ Objective-C and Objective-C++ Dialects}.
> -Wsystem-headers -Wtautological-compare -Wtrampolines -Wtrigraphs
> -Wtrivial-auto-var-init -Wtsan -Wtype-limits -Wundef
> -Wuninitialized -Wunknown-pragmas
> --Wunsuffixed-float-constants -Wunused
> +-Wunsuffixed-float-constants
> +-Wunterminated-string-initialization
> +-Wunused
> -Wunused-but-set-parameter -Wunused-but-set-variable
> -Wunused-const-variable -Wunused-const-variable=@var{n}
> -Wunused-function -Wunused-label -Wunused-local-typedefs
> @@ -6264,6 +6266,7 @@ name is still supported, but the newer name is more
> descriptive.)
> -Wredundant-move @r{(only for C++)}
> -Wtype-limits
> -Wuninitialized
> +-Wunterminated-string-initialization
> -Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}
> -Wunused-parameter @r{(only with} @option{-Wunused} @r{or}
> @option{-Wall}@r{)}
> -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or}
> @option{-Wall}@r{)}}
> @@ -8281,6 +8284,20 @@ arithmetic that may yield out of bounds values. This
> warning level may
> give a larger number of false positives and is deactivated by default.
> @end table
>
> +@opindex Wunterminated-string-initialization
> +@opindex Wno-unterminated-string-initialization
> +@item -Wunterminated-string-initialization
> +Warn about character arrays
> +initialized as unterminated character sequences
> +with a string literal.
> +For example:
> +
> +@smallexample
> +char arr[3] = "foo";
> +@end smallexample
> +
> +@option{-Wunterminated-string-initialization} is enabled by @option{-Wextra}.
Oops, I should also mention -Wc++-compat here.
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
signature.asc
Description: PGP signature
[PATCH v6] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Since Wc++-compat now includes this warning, the test has to be modified
to expect the text of the new warning too, in .
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Cc: Mike Stump
Cc: Joseph Myers
Cc: Sandra Loosemore
Signed-off-by: Alejandro Colomar
---
Hi!
v6:
- Small wording fix in c.opt
- Document the option in invoke.texi
I tried again, but didn't find much alphabetic order in there, so put
it where Mike suggested, after -Warray-bounds=n.
Have a lovely night!
Alex
Range-diff against v5:
1: d98d1fec176 ! 1: e8fd975bde7 C, ObjC: Add
-Wunterminated-string-initialization
@@ gcc/c-family/c.opt: Wunsuffixed-float-constants
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning
LangEnabledBy(C ObjC,Wextra || Wc++-compat)
-+Warn about character arrays initialized as unterminated character
sequences by a string literal.
++Warn about character arrays initialized as unterminated character
sequences with a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
@@ gcc/c/c-typeck.cc: digest_init (location_t init_loc, tree type, tree
init, tree
{
unsigned HOST_WIDE_INT size
+ ## gcc/doc/invoke.texi ##
+@@ gcc/doc/invoke.texi: Objective-C and Objective-C++ Dialects}.
+ -Wsystem-headers -Wtautological-compare -Wtrampolines -Wtrigraphs
+ -Wtrivial-auto-var-init -Wtsan -Wtype-limits -Wundef
+ -Wuninitialized -Wunknown-pragmas
+--Wunsuffixed-float-constants -Wunused
++-Wunsuffixed-float-constants
++-Wunterminated-string-initialization
++-Wunused
+ -Wunused-but-set-parameter -Wunused-but-set-variable
+ -Wunused-const-variable -Wunused-const-variable=@var{n}
+ -Wunused-function -Wunused-label -Wunused-local-typedefs
+@@ gcc/doc/invoke.texi: name is still supported, but the newer name is
more descriptive.)
+ -Wredundant-move @r{(only for C++)}
+ -Wtype-limits
+ -Wuninitialized
++-Wunterminated-string-initialization
+ -Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}
+ -Wunused-parameter @r{(only with} @option{-Wunused} @r{or}
@option{-Wall}@r{)}
+ -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or}
@option{-Wall}@r{)}}
+@@ gcc/doc/invoke.texi: arithmetic that may yield out of bounds values.
This warning level may
+ give a larger number of false positives and is deactivated by default.
+ @end table
+
++@opindex Wunterminated-string-initialization
++@opindex Wno-unterminated-string-initialization
++@item -Wunterminated-string-initialization
++Warn about character arrays
++initialized as unterminated character sequences
++with a string literal.
++For example:
++
++@smallexample
++char arr[3] = "foo";
++@end smallexample
++
++@option{-Wunterminated-string-initialization} is enabled by
@option{-Wextra}.
++
+ @opindex Warray-compare
+ @opindex Wno-array-compare
+ @item -Warray-compare
+
## gcc/testsuite/gcc.dg/Wcxx-com
Re: [PATCH v5 RESEND] C, ObjC: Add -Wunterminated-string-initialization
Hi Joseph,
On Mon, Feb 26, 2024 at 03:24:32PM +, Joseph Myers wrote:
> On Sun, 25 Feb 2024, Mike Stump wrote:
>
> > On Feb 6, 2024, at 2:45 AM, Alejandro Colomar wrote:
> > >
> > > Warn about the following:
> > >
> > >char s[3] = "foo";
> >
> > No ObjC specific impact here, so no need for ObjC review.
> >
> > As a member of the peanut gallery, I like the patch.
> >
> > Joseph, this is been submitted 5 times over the past year. Any thoughts?
>
> The idea seems reasonable, but the patch needs documentation for the new
> option in invoke.texi.
Thanks! Will do.
I don't see an obvious order in that file. Where would you put the
option? Do you want me to sort(1) it first, and then insert the new
option in alphabetic order?
$ man gcc 2>/dev/null | grep -e '^ \{0,3\}[^ ]' -e '^ \{4,7\}-' | sed -n '/^
\{1,3\}Options to Request or Suppress Warnings/,/^ \{1,3\}[^ ]/p'
Options to Request or Suppress Warnings
-fsyntax-only
-fmax-errors=n
-w Inhibit all warning messages.
-Werror
-Werror=
-Wfatal-errors
-Wno- options with old compilers, but if something goes wrong, the
compiler warns that an unrecognized option is present.
-Wpedantic
-pedantic
-pedantic-errors
-Wall
-Wextra
-Wabi (C, Objective‐C, C++ and Objective-C++ only)
-Wno-changes-meaning (C++ and Objective-C++ only)
-Wchar-subscripts
-Wno-coverage-mismatch
-Wno-coverage-invalid-line-number
-Wno-cpp (C, Objective‐C, C++, Objective-C++ and Fortran only)
-Wdouble-promotion (C, C++, Objective‐C and Objective-C++ only)
-Wduplicate-decl-specifier (C and Objective‐C only)
-Wformat
-Wformat=n
-Wno-format-contains-nul
-Wno-format-extra-args
-Wformat-overflow
-Wformat-overflow=level
-Wno-format-zero-length
-Wformat-nonliteral
-Wformat-security
-Wformat-signedness
-Wformat-truncation
-Wformat-truncation=level
-Wformat-y2k
-Wnonnull
-Wnonnull-compare
-Wnull-dereference
-Winfinite-recursion
-Winit-self (C, C++, Objective‐C and Objective-C++ only)
-Wno-implicit-int (C and Objective‐C only)
-Wno-implicit-function-declaration (C and Objective‐C only)
-Wimplicit (C and Objective‐C only)
-Wimplicit-fallthrough
-Wimplicit-fallthrough=n
-Wno-if-not-aligned (C, C++, Objective‐C and Objective-C++ only)
-Wignored-qualifiers (C and C++ only)
-Wno-ignored-attributes (C and C++ only)
-Wmain
-Wmisleading-indentation (C and C++ only)
-Wmissing-attributes
-Wmissing-braces
-Wmissing-include-dirs (C, C++, Objective‐C, Objective-C++ and Fortran
only)
-Wno-missing-profile
-Wmismatched-dealloc
-Wmultistatement-macros
-Wparentheses
-Wno-self-move (C++ and Objective-C++ only)
-Wsequence-point
-Wno-return-local-addr
-Wreturn-type
-Wno-shift-count-negative
-Wno-shift-count-overflow
-Wshift-negative-value
-Wno-shift-overflow
-Wshift-overflow=n
-Wswitch
-Wswitch-default
-Wswitch-enum
-Wno-switch-bool
-Wno-switch-outside-range
-Wno-switch-unreachable
-Wsync-nand (C and C++ only)
-Wtrivial-auto-var-init
-Wunused-but-set-parameter
-Wunused-but-set-variable
-Wunused-function
-Wunused-label
-Wunused-local-typedefs (C, Objective‐C, C++ and Objective-C++ only)
-Wunused-parameter
-Wno-unused-result
-Wunused-variable
-Wunused-const-variable
-Wunused-const-variable=n
-Wunused-value
-Wunused
-Wuninitialized
-Wno-invalid-memory-model
-Wmaybe-uninitialized
-Wunknown-pragmas
-Wno-pragmas
-Wno-prio-ctor-dtor
-Wstrict-aliasing
-Wstrict-aliasing=n
-Wstrict-overflow
-Wstrict-overflow=n
-Wstring-compare
-Wno-stringop-overflow
-Wstringop-overflow
-Wstringop-overflow=type
-Wno-stringop-overread
-Wno-stringop-truncation
-Wstrict-flex-arrays
-Wsuggest-attribute=[pure|const|noreturn|format|cold|malloc]
-Walloc-zero
-Walloc-size-larger-than=byte‐size
-Wno-alloc-size-larger-than
-Walloca
-Walloca-larger-than=byte‐size
-Wno-alloca-larger-than
-Warith-conversion
-Warray-bounds
-Warray-bounds=n
-Warray-compare
-Warray-parameter
-Warray-parameter=n
-Wattribute-alias=n
-Wno-attribute-alias
-Wbidi-chars=[none|unpaired|any|ucn]
-Wbool-compare
-Wbool-operation
-Wduplicated-branches
-Wduplicated-cond
-Wframe-address
-Wno-discarded-qualifiers (C and Objective‐C only)
-Wno-discarded-array-qualifiers (C and Objective‐C only)
-Wno-incompatible-pointer-types (C and Objective‐C only)
-Wno-int-conversion (C and Objective‐C only)
-Wzero-length-bounds
-Wno-div-by-zero
Re: [PATCH v5 RESEND] C, ObjC: Add -Wunterminated-string-initialization
Hi Mike, Joseph, On Sun, Feb 25, 2024 at 10:10:09AM -0800, Mike Stump wrote: > On Feb 6, 2024, at 2:45 AM, Alejandro Colomar wrote: > > > > Warn about the following: > > > >char s[3] = "foo"; > > No ObjC specific impact here, so no need for ObjC review. > > As a member of the peanut gallery, I like the patch. > > Joseph, this is been submitted 5 times over the past year. Any thoughts? Thanks! BTW, I'd like to know if I did anything wrong so that it wasn't reviewed in all this time, or if it's just that everyone was busy doing other stuff. Do you prefer if I ping more often? Or something else? Have a lovely day! Alex -- <https://www.alejandro-colomar.es/> Looking for a remote C programming job at the moment. signature.asc Description: PGP signature
[PATCH v5 RESEND] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Since Wc++-compat now includes this warning, the test has to be modified
to expect the text of the new warning too, in .
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Signed-off-by: Alejandro Colomar
---
v5:
- Fix existing C++-compat tests. [reported by ]
gcc/c-family/c.opt | 4
gcc/c/c-typeck.cc | 6 +++---
gcc/testsuite/gcc.dg/Wcxx-compat-14.c | 2 +-
gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
4 files changed, 14 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44b9c862c14..e8f6b836836 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
C ObjC Var(warn_unsuffixed_float_constants) Warning
Warn about unsuffixed float constants.
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences by
a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e55e887da14..7df9de819ed 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree init,
tree origtype,
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
- else if (warn_cxx_compat
+ else if (warn_unterminated_string_initialization
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wc___compat,
+ warning_at (init_loc, OPT_Wunterminated_string_initialization,
("initializer-string for array of %qT "
-"is too long for C++"), typ1);
+"is too long"), typ1);
if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
{
unsigned HOST_WIDE_INT size
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
index 23783711be6..6df0ee197cc 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
@@ -2,5 +2,5 @@
/* { dg-options "-Wc++-compat" } */
char a1[] = "a";
-char a2[1] = "a"; /* { dg-warning "C\[+\]\[+\]" } */
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
char a3[2] = "a";
diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
new file mode 100644
index 000..13d5dbc6640
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initi
Re: [PATCH v5] C, ObjC: Add -Wunterminated-string-initialization
Hi,
Gentle ping, just again a little before v14 stage 3.
Do I need to do anything else with this patch? The CI seemed to say
it's ok.
Cheers,
Alex
On Sun, Oct 01, 2023 at 06:24:00PM +0200, Alejandro Colomar wrote:
> Warn about the following:
>
> char s[3] = "foo";
>
> Initializing a char array with a string literal of the same length as
> the size of the array is usually a mistake. Rarely is the case where
> one wants to create a non-terminated character sequence from a string
> literal.
>
> In some cases, for writing faster code, one may want to use arrays
> instead of pointers, since that removes the need for storing an array of
> pointers apart from the strings themselves.
>
> char *log_levels[] = { "info", "warning", "err" };
> vs.
> char log_levels[][7] = { "info", "warning", "err" };
>
> This forces the programmer to specify a size, which might change if a
> new entry is later added. Having no way to enforce null termination is
> very dangerous, however, so it is useful to have a warning for this, so
> that the compiler can make sure that the programmer didn't make any
> mistakes. This warning catches the bug above, so that the programmer
> will be able to fix it and write:
>
> char log_levels[][8] = { "info", "warning", "err" };
>
> This warning already existed as part of -Wc++-compat, but this patch
> allows enabling it separately. It is also included in -Wextra, since
> it may not always be desired (when unterminated character sequences are
> wanted), but it's likely to be desired in most cases.
>
> Since Wc++-compat now includes this warning, the test has to be modified
> to expect the text of the new warning too, in .
>
> Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
> Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
> Link:
> <https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
> Acked-by: Doug McIlroy
> Cc: "G. Branden Robinson"
> Cc: Ralph Corderoy
> Cc: Dave Kemper
> Cc: Larry McVoy
> Cc: Andrew Pinski
> Cc: Jonathan Wakely
> Cc: Andrew Clayton
> Cc: Martin Uecker
> Cc: David Malcolm
> Signed-off-by: Alejandro Colomar
> ---
>
> v5:
>
> - Fix existing C++-compat tests. [reported by ]
>
>
> gcc/c-family/c.opt | 4
> gcc/c/c-typeck.cc | 6 +++---
> gcc/testsuite/gcc.dg/Wcxx-compat-14.c | 2 +-
> gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
> 4 files changed, 14 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 44b9c862c14..e8f6b836836 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
> C ObjC Var(warn_unsuffixed_float_constants) Warning
> Warn about unsuffixed float constants.
>
> +Wunterminated-string-initialization
> +C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
> ObjC,Wextra || Wc++-compat)
> +Warn about character arrays initialized as unterminated character sequences
> by a string literal.
> +
> Wunused
> C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
> ; documented in common.opt
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index e55e887da14..7df9de819ed 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree
> init, tree origtype,
> pedwarn_init (init_loc, 0,
> ("initializer-string for array of %qT "
> "is too long"), typ1);
> - else if (warn_cxx_compat
> + else if (warn_unterminated_string_initialization
> && compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
> - warning_at (init_loc, OPT_Wc___compat,
> + warning_at (init_loc, OPT_Wunterminated_string_initialization,
> ("initializer-string for array of %qT "
> - "is too long for C++"), typ1);
> + "is too long"), typ1);
> if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
> {
> unsigned HOST_WIDE_INT size
> diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
> b/gcc/testsuite/gcc.dg/Wcxx-compat-
Ping: [PATCH v5] C, ObjC: Add -Wunterminated-string-initialization
Hi,
Gentle ping here.
Thanks,
Alex
On Sun, Oct 01, 2023 at 06:24:00PM +0200, Alejandro Colomar wrote:
> Warn about the following:
>
> char s[3] = "foo";
>
> Initializing a char array with a string literal of the same length as
> the size of the array is usually a mistake. Rarely is the case where
> one wants to create a non-terminated character sequence from a string
> literal.
>
> In some cases, for writing faster code, one may want to use arrays
> instead of pointers, since that removes the need for storing an array of
> pointers apart from the strings themselves.
>
> char *log_levels[] = { "info", "warning", "err" };
> vs.
> char log_levels[][7] = { "info", "warning", "err" };
>
> This forces the programmer to specify a size, which might change if a
> new entry is later added. Having no way to enforce null termination is
> very dangerous, however, so it is useful to have a warning for this, so
> that the compiler can make sure that the programmer didn't make any
> mistakes. This warning catches the bug above, so that the programmer
> will be able to fix it and write:
>
> char log_levels[][8] = { "info", "warning", "err" };
>
> This warning already existed as part of -Wc++-compat, but this patch
> allows enabling it separately. It is also included in -Wextra, since
> it may not always be desired (when unterminated character sequences are
> wanted), but it's likely to be desired in most cases.
>
> Since Wc++-compat now includes this warning, the test has to be modified
> to expect the text of the new warning too, in .
>
> Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
> Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
> Link:
> <https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
> Acked-by: Doug McIlroy
> Cc: "G. Branden Robinson"
> Cc: Ralph Corderoy
> Cc: Dave Kemper
> Cc: Larry McVoy
> Cc: Andrew Pinski
> Cc: Jonathan Wakely
> Cc: Andrew Clayton
> Cc: Martin Uecker
> Cc: David Malcolm
> Signed-off-by: Alejandro Colomar
> ---
>
> v5:
>
> - Fix existing C++-compat tests. [reported by ]
>
>
> gcc/c-family/c.opt | 4
> gcc/c/c-typeck.cc | 6 +++---
> gcc/testsuite/gcc.dg/Wcxx-compat-14.c | 2 +-
> gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
> 4 files changed, 14 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 44b9c862c14..e8f6b836836 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
> C ObjC Var(warn_unsuffixed_float_constants) Warning
> Warn about unsuffixed float constants.
>
> +Wunterminated-string-initialization
> +C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
> ObjC,Wextra || Wc++-compat)
> +Warn about character arrays initialized as unterminated character sequences
> by a string literal.
> +
> Wunused
> C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
> ; documented in common.opt
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index e55e887da14..7df9de819ed 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree
> init, tree origtype,
> pedwarn_init (init_loc, 0,
> ("initializer-string for array of %qT "
> "is too long"), typ1);
> - else if (warn_cxx_compat
> + else if (warn_unterminated_string_initialization
> && compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
> - warning_at (init_loc, OPT_Wc___compat,
> + warning_at (init_loc, OPT_Wunterminated_string_initialization,
> ("initializer-string for array of %qT "
> - "is too long for C++"), typ1);
> + "is too long"), typ1);
> if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
> {
> unsigned HOST_WIDE_INT size
> diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
> b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
> index 23783711be6..6df0ee197cc 100644
> --- a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
> +++ b/gcc/testsuite/
[PATCH v5] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Since Wc++-compat now includes this warning, the test has to be modified
to expect the text of the new warning too, in .
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Signed-off-by: Alejandro Colomar
---
v5:
- Fix existing C++-compat tests. [reported by ]
gcc/c-family/c.opt | 4
gcc/c/c-typeck.cc | 6 +++---
gcc/testsuite/gcc.dg/Wcxx-compat-14.c | 2 +-
gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
4 files changed, 14 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44b9c862c14..e8f6b836836 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
C ObjC Var(warn_unsuffixed_float_constants) Warning
Warn about unsuffixed float constants.
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences by
a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e55e887da14..7df9de819ed 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree init,
tree origtype,
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
- else if (warn_cxx_compat
+ else if (warn_unterminated_string_initialization
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wc___compat,
+ warning_at (init_loc, OPT_Wunterminated_string_initialization,
("initializer-string for array of %qT "
-"is too long for C++"), typ1);
+"is too long"), typ1);
if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
{
unsigned HOST_WIDE_INT size
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
index 23783711be6..6df0ee197cc 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
@@ -2,5 +2,5 @@
/* { dg-options "-Wc++-compat" } */
char a1[] = "a";
-char a2[1] = "a"; /* { dg-warning "C\[+\]\[+\]" } */
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
char a3[2] = "a";
diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
new file mode 100644
index 000..13d5dbc6640
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunterminated-string-initialization" } */
+
+char a1[] = "a";
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a3[2] = "a";
--
2.40.1
[PATCH v4] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Signed-off-by: Alejandro Colomar
---
v4:
- Fix From: address
gcc/c-family/c.opt | 4
gcc/c/c-typeck.cc | 6 +++---
gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
3 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44b9c862c14..e8f6b836836 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
C ObjC Var(warn_unsuffixed_float_constants) Warning
Warn about unsuffixed float constants.
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences by
a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e55e887da14..7df9de819ed 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree init,
tree origtype,
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
- else if (warn_cxx_compat
+ else if (warn_unterminated_string_initialization
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wc___compat,
+ warning_at (init_loc, OPT_Wunterminated_string_initialization,
("initializer-string for array of %qT "
-"is too long for C++"), typ1);
+"is too long"), typ1);
if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
{
unsigned HOST_WIDE_INT size
diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
new file mode 100644
index 000..13d5dbc6640
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunterminated-string-initialization" } */
+
+char a1[] = "a";
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a3[2] = "a";
--
2.40.1
[PATCH v3] C, ObjC: Add -Wunterminated-string-initialization
From: Alejandro Colomar
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Cc: David Malcolm
Signed-off-by: Alejandro Colomar
---
v3:
- Fix dg-warning message in the testsuite.
gcc/c-family/c.opt | 4
gcc/c/c-typeck.cc | 6 +++---
gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
3 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44b9c862c14..e8f6b836836 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1407,6 +1407,10 @@ Wunsuffixed-float-constants
C ObjC Var(warn_unsuffixed_float_constants) Warning
Warn about unsuffixed float constants.
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences by
a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e55e887da14..7df9de819ed 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8399,11 +8399,11 @@ digest_init (location_t init_loc, tree type, tree init,
tree origtype,
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
- else if (warn_cxx_compat
+ else if (warn_unterminated_string_initialization
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wc___compat,
+ warning_at (init_loc, OPT_Wunterminated_string_initialization,
("initializer-string for array of %qT "
-"is too long for C++"), typ1);
+"is too long"), typ1);
if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
{
unsigned HOST_WIDE_INT size
diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
new file mode 100644
index 000..13d5dbc6640
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunterminated-string-initialization" } */
+
+char a1[] = "a";
+char a2[1] = "a"; /* { dg-warning "initializer-string for array of 'char'
is too long" } */
+char a3[2] = "a";
--
2.40.1
Re: [PATCH v2] C, ObjC: Add -Wunterminated-string-initialization
On Sun, Oct 01, 2023 at 09:37:59AM +0200, Martin Uecker wrote:
>
> (I shortened the recipient list)
>
> Am Sonntag, dem 01.10.2023 um 02:55 +0200 schrieb Alejandro Colomar:
>
> > >
> ...
> > I ran the tests, and get some unexpected failure. I used dg-warning,
> > but maybe I used it wrong? Here's the output:
> >
> > ```
> > output is:
> > /home/alx/src/gnu/gcc/wustr/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c:5:14:
> > warning: initializer-string for array of 'char' is too long
> > [-Wunterminated-string-initi
> > alization]
> >
> > FAIL: gcc.dg/Wunterminated-string-initialization.c (test for warnings,
> > line 5)
> > ```
> >
> > And here's the test:
> >
> > ```
> > /* { dg-do compile } */
> > /* { dg-options "-Wunterminated-string-initialization" } */
> >
> > char a1[] = "a";
> > char a2[1] = "a"; /* { dg-warning "unterminated char sequence" } */
> > char a3[2] = "a";
> > ```
> >
> > Why isn't it expecting the warning?
>
> Because the text does not match the actual output above?
Makes sense.
Here's the output after a fix (which I'll send soon as a new revision):
```
output is:
/home/alx/src/gnu/gcc/wustr/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c:5:14:
warning: initializer-string for array of 'char' is too long
[-Wunterminated-string-initialization]
```
and
```
=== gcc Summary ===
# of expected passes2
```
The only thing that concerns me a little bit is that I don't see any
PASS (or XFAIL).
> You should see an additional FAIL for an excess warning.
Yep, I did.
Cheers,
Alex
>
> Martin
>
>
>
>
signature.asc
Description: PGP signature
Re: [PATCH v2] C, ObjC: Add -Wunterminated-string-initialization
Hi David,
Sorry for the half-year delay! I have some update. :)
On Fri, Mar 24, 2023 at 10:53:22AM -0400, David Malcolm wrote:
> On Fri, 2023-03-24 at 14:39 +0100, Alejandro Colomar via Gcc-patches
> wrote:
> > Warn about the following:
> >
> > char s[3] = "foo";
> >
> > Initializing a char array with a string literal of the same length as
> > the size of the array is usually a mistake. Rarely is the case where
> > one wants to create a non-terminated character sequence from a string
> > literal.
> >
> > In some cases, for writing faster code, one may want to use arrays
> > instead of pointers, since that removes the need for storing an array
> > of
> > pointers apart from the strings themselves.
> >
> > char *log_levels[] = { "info", "warning", "err" };
> > vs.
> > char log_levels[][7] = { "info", "warning", "err" };
> >
> > This forces the programmer to specify a size, which might change if a
> > new entry is later added. Having no way to enforce null termination
> > is
> > very dangerous, however, so it is useful to have a warning for this,
> > so
> > that the compiler can make sure that the programmer didn't make any
> > mistakes. This warning catches the bug above, so that the programmer
> > will be able to fix it and write:
> >
> > char log_levels[][8] = { "info", "warning", "err" };
> >
> > This warning already existed as part of -Wc++-compat, but this patch
> > allows enabling it separately. It is also included in -Wextra, since
> > it may not always be desired (when unterminated character sequences
> > are
> > wanted), but it's likely to be desired in most cases.
> >
> > Link:
> > <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
> > Link:
> > <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
> > Link:
> > <https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf41
> > 3...@gmail.com/T/>
> > Acked-by: Doug McIlroy
> > Cc: "G. Branden Robinson"
> > Cc: Ralph Corderoy
> > Cc: Dave Kemper
> > Cc: Larry McVoy
> > Cc: Andrew Pinski
> > Cc: Jonathan Wakely
> > Cc: Andrew Clayton
> > Cc: Martin Uecker
> > Signed-off-by: Alejandro Colomar
> > ---
> >
> > Hi,
>
> Hi Alex, thanks for the patch.
>
> >
> > I sent v1 to the wrong list. This time I've made sure to write to
> > gcc-patches@.
>
> Note that right now we're deep in bug-fixing/stabilization for GCC 13
> (and trunk is still tracking that effort), so your patch might be more
> suitable for GCC 14.
>
> >
> > v2 adds some draft of a test, as suggested by Martin. However, I
> > don't
> > know yet how to write those, so the test is just a draft. But I did
> > test the feature, by compiling GCC and compiling some small program
> > with
> > it.
>
> Unfortunately the answer to the question "how do I run just one
> testcase in GCC's testsuite" is rather non-trivial; FWIW I've written
> up some notes on working with the GCC testsuite here:
> https://gcc-newbies-guide.readthedocs.io/en/latest/working-with-the-testsuite.html
>
> Hope this is helpful
> Dave
>
>
> [...snip...]
>
I ran the tests, and get some unexpected failure. I used dg-warning,
but maybe I used it wrong? Here's the output:
```
output is:
/home/alx/src/gnu/gcc/wustr/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c:5:14:
warning: initializer-string for array of 'char' is too long
[-Wunterminated-string-initi
alization]
FAIL: gcc.dg/Wunterminated-string-initialization.c (test for warnings, line 5)
```
And here's the test:
```
/* { dg-do compile } */
/* { dg-options "-Wunterminated-string-initialization" } */
char a1[] = "a";
char a2[1] = "a"; /* { dg-warning "unterminated char sequence" } */
char a3[2] = "a";
```
Why isn't it expecting the warning?
Thanks,
Alex
signature.asc
Description: PGP signature
[PATCH] doc: tfix
Remove repeated word (typo).
Signed-off-by: Alejandro Colomar
---
gcc/doc/extend.texi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index fd3745c5608..cdfb25ff272 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3756,7 +3756,7 @@ take function pointer arguments.
The @code{optimize} attribute is used to specify that a function is to
be compiled with different optimization options than specified on the
command line. The optimize attribute arguments of a function behave
-behave as if appended to the command-line.
+as if appended to the command-line.
Valid arguments are constant non-negative integers and
strings. Each numeric argument specifies an optimization @var{level}.
--
2.40.0
Ping: [PATCH v2] C, ObjC: Add -Wunterminated-string-initialization
Hi David, On 3/24/23 18:58, David Malcolm wrote: > On Fri, 2023-03-24 at 18:45 +0100, Alejandro Colomar wrote: >> Hi David, >> >> On 3/24/23 15:53, David Malcolm wrote: >>> On Fri, 2023-03-24 at 14:39 +0100, Alejandro Colomar via Gcc- >>> patches >>> wrote: >>>> Warn about the following: >>>> >>>> char s[3] = "foo"; >>>> >> [...] >> >>>> --- >>>> >>>> Hi, >>> >>> Hi Alex, thanks for the patch. >> >> :) >> >>> >>>> >>>> I sent v1 to the wrong list. This time I've made sure to write >>>> to >>>> gcc-patches@. >>> >>> Note that right now we're deep in bug-fixing/stabilization for GCC >>> 13 >>> (and trunk is still tracking that effort), so your patch might be >>> more >>> suitable for GCC 14. >> >> Sure, no problem. Do you have a "next" branch where you pick patches >> for after the release, or should I resend after the release? > > We don't; resending it after release is probably best. > >> Is >> discussion of a patch reasonable now, or is it too much distracting >> from your stabilization efforts? > > FWIW I'd prefer to postpone the discussion until after we branch for > the release. Sure. AFAIK it's fair game already to propose patches to GCC 14, right? Would you please have a look into this? Thanks! > >> >>> >>>> >>>> v2 adds some draft of a test, as suggested by Martin. However, I >>>> don't >>>> know yet how to write those, so the test is just a draft. But I >>>> did >>>> test the feature, by compiling GCC and compiling some small >>>> program >>>> with >>>> it. >>> >>> Unfortunately the answer to the question "how do I run just one >>> testcase in GCC's testsuite" is rather non-trivial; FWIW I've >>> written >>> up some notes on working with the GCC testsuite here: >>> https://gcc-newbies-guide.readthedocs.io/en/latest/working-with-the-testsuite.html >> >> Hmm, I'll try following that; thanks! Is there anything obvious that >> I might have missed, at first glance? > > The main thing is that there's a difference between compiling the test > case "by hand", versus doing it through the test harness - the latter > sets up the environment in a particular way, injects a particular set > of flags, etc etc. I forgot about this; I'll have a look into it when I find some time. Cheers, Alex > > Dave > -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v2] C, ObjC: Add -Wunterminated-string-initialization
Hi David, On 3/24/23 15:53, David Malcolm wrote: > On Fri, 2023-03-24 at 14:39 +0100, Alejandro Colomar via Gcc-patches > wrote: >> Warn about the following: >> >> char s[3] = "foo"; >> [...] >> --- >> >> Hi, > > Hi Alex, thanks for the patch. :) > >> >> I sent v1 to the wrong list. This time I've made sure to write to >> gcc-patches@. > > Note that right now we're deep in bug-fixing/stabilization for GCC 13 > (and trunk is still tracking that effort), so your patch might be more > suitable for GCC 14. Sure, no problem. Do you have a "next" branch where you pick patches for after the release, or should I resend after the release? Is discussion of a patch reasonable now, or is it too much distracting from your stabilization efforts? > >> >> v2 adds some draft of a test, as suggested by Martin. However, I >> don't >> know yet how to write those, so the test is just a draft. But I did >> test the feature, by compiling GCC and compiling some small program >> with >> it. > > Unfortunately the answer to the question "how do I run just one > testcase in GCC's testsuite" is rather non-trivial; FWIW I've written > up some notes on working with the GCC testsuite here: > https://gcc-newbies-guide.readthedocs.io/en/latest/working-with-the-testsuite.html Hmm, I'll try following that; thanks! Is there anything obvious that I might have missed, at first glance? > > Hope this is helpful Yup. Thanks! > Dave Cheers, Alex -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 OpenPGP_signature Description: OpenPGP digital signature
[PATCH v2] C, ObjC: Add -Wunterminated-string-initialization
Warn about the following:
char s[3] = "foo";
Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake. Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.
In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.
char *log_levels[] = { "info", "warning", "err" };
vs.
char log_levels[][7] = { "info", "warning", "err" };
This forces the programmer to specify a size, which might change if a
new entry is later added. Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes. This warning catches the bug above, so that the programmer
will be able to fix it and write:
char log_levels[][8] = { "info", "warning", "err" };
This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately. It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link:
<https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf...@gmail.com/T/>
Acked-by: Doug McIlroy
Cc: "G. Branden Robinson"
Cc: Ralph Corderoy
Cc: Dave Kemper
Cc: Larry McVoy
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
Cc: Martin Uecker
Signed-off-by: Alejandro Colomar
---
Hi,
I sent v1 to the wrong list. This time I've made sure to write to
gcc-patches@.
v2 adds some draft of a test, as suggested by Martin. However, I don't
know yet how to write those, so the test is just a draft. But I did
test the feature, by compiling GCC and compiling some small program with
it.
Cheers,
Alex
Range-diff against v1:
1: 61ddf1eb816 ! 1: e40d8f54942 C, ObjC: Add
-Wunterminated-string-initialization
@@ Commit message
Cc: Andrew Pinski
Cc: Jonathan Wakely
Cc: Andrew Clayton
+Cc: Martin Uecker
Signed-off-by: Alejandro Colomar
## gcc/c-family/c.opt ##
@@ gcc/c/c-typeck.cc: digest_init (location_t init_loc, tree type, tree
init, tree
if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
{
unsigned HOST_WIDE_INT size
+
+ ## gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c (new) ##
+@@
++/* { dg-do compile } */
++/* { dg-options "-Wunterminated-string-initialization" } */
++
++char a1[] = "a";
++char a2[1] = "a"; /* { dg-warning "unterminated char sequence" } */
++char a3[2] = "a";
gcc/c-family/c.opt | 4
gcc/c/c-typeck.cc | 6 +++---
gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++
3 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index cddeece..7f1fccfe02b 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1382,6 +1382,10 @@ Wunsuffixed-float-constants
C ObjC Var(warn_unsuffixed_float_constants) Warning
Warn about unsuffixed float constants.
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences by
a string literal.
+
Wunused
C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 45bacc06c47..ce2750f98bb 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8420,11 +8420,11 @@ digest_init (location_t init_loc, tree type, tree init,
tree origtype,
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
- else if (warn_cxx_compat
+ else if (warn_unterminated_string_initialization
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
- warning_at (init_loc, OPT_Wc___compat,
+ warning_at (init_loc, OPT_Wunterminated_string_initialization,
("initializer-string for array of %qT "
-"is too long for C++"), typ1);
+
Re: Ping: [PATCH resend] Make -Wuse-after-free=3 the default one in -Wall
Hi Richard, On 3/15/23 15:52, Richard Biener wrote: > On Wed, Mar 15, 2023 at 3:30 PM Alejandro Colomar via Gcc-patches > wrote: >> >> Ping > > -Wuse-after-free=3 was explicitly added to cover cases with a high > false-positive rate. If you want to > make that the default then instead merge the equality compare case > back to the =2 case. > > But as I said elsewhere I think that -Wuse-after-free is very much too > trigger happy, especially > with value-uses (not accessing released memory but inspecting the old > pointer value). Please consider > looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104075 and > review the false positives > reported. > > Also see my very recent patches from today trying to limit > -Wuse-after-free by not diagnosing > from late IL. Hmmm, thanks, didn't know about those. Please ignore my patch. Cheers, Alex -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5 OpenPGP_signature Description: OpenPGP digital signature
Ping: [PATCH resend] Make -Wuse-after-free=3 the default one in -Wall
Ping
On 2/18/23 00:05, Alejandro Colomar wrote:
> Link:
> <https://inbox.sourceware.org/gcc/3098fd18-9dbf-b4e9-bae5-62ec6fea7...@opteya.com/T/>
> Link: <https://github.com/shadow-maint/shadow/pull/649#discussion_r1108350066>
> Cc: Andreas Schwab
> Cc: David Malcolm
> Cc: Florian Weimer
> Cc: Iker Pedrosa
> Cc: Jens Gustedt
> Cc: Jonathan Wakely
> Cc: Mark Wielaard
> Cc: Martin Uecker
> Cc: Michael Kerrisk
> Cc: Paul Eggert
> Cc: Sam James
> Cc: Siddhesh Poyarekar
> Cc: Yann Droneaud
> Signed-off-by: Alejandro Colomar
> ---
>
> This is a resend of the same patch previously sent to gcc@.
>
> gcc/c-family/c.opt | 4 ++--
> gcc/doc/invoke.texi | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index c0fea56a8f5..1a3fc2c5d74 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1411,11 +1411,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger
> Var(warn_unused_const_variable)
> Warn when a const variable is unused.
>
> ; Defining this option here in addition to common.opt is necessary
> -; in order for the default -Wall setting of -Wuse-after-free=2 to take
> +; in order for the default -Wall setting of -Wuse-after-free=3 to take
> ; effect.
>
> Wuse-after-free=
> -LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0)
> +LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,3,0)
> ; in common.opt
>
> Wvariadic-macros
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 7b308cd3c31..d910052ce0c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -4720,7 +4720,7 @@ instead of pointers. This approach obviates needing to
> adjust the stored
> pointers after reallocation.
> @end table
>
> -@option{-Wuse-after-free=2} is included in @option{-Wall}.
> +@option{-Wuse-after-free=3} is included in @option{-Wall}.
>
> @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
> @opindex Wuseless-cast
--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH resend] Make -Wuse-after-free=3 the default one in -Wall
Link:
<https://inbox.sourceware.org/gcc/3098fd18-9dbf-b4e9-bae5-62ec6fea7...@opteya.com/T/>
Link: <https://github.com/shadow-maint/shadow/pull/649#discussion_r1108350066>
Cc: Andreas Schwab
Cc: David Malcolm
Cc: Florian Weimer
Cc: Iker Pedrosa
Cc: Jens Gustedt
Cc: Jonathan Wakely
Cc: Mark Wielaard
Cc: Martin Uecker
Cc: Michael Kerrisk
Cc: Paul Eggert
Cc: Sam James
Cc: Siddhesh Poyarekar
Cc: Yann Droneaud
Signed-off-by: Alejandro Colomar
---
This is a resend of the same patch previously sent to gcc@.
gcc/c-family/c.opt | 4 ++--
gcc/doc/invoke.texi | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c0fea56a8f5..1a3fc2c5d74 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1411,11 +1411,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger
Var(warn_unused_const_variable)
Warn when a const variable is unused.
; Defining this option here in addition to common.opt is necessary
-; in order for the default -Wall setting of -Wuse-after-free=2 to take
+; in order for the default -Wall setting of -Wuse-after-free=3 to take
; effect.
Wuse-after-free=
-LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0)
+LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,3,0)
; in common.opt
Wvariadic-macros
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7b308cd3c31..d910052ce0c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4720,7 +4720,7 @@ instead of pointers. This approach obviates needing to
adjust the stored
pointers after reallocation.
@end table
-@option{-Wuse-after-free=2} is included in @option{-Wall}.
+@option{-Wuse-after-free=3} is included in @option{-Wall}.
@item -Wuseless-cast @r{(C++ and Objective-C++ only)}
@opindex Wuseless-cast
--
2.39.1
Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming
On 8/25/22 09:44, Alejandro Colomar wrote: Hi Greg, On 8/25/22 07:57, Greg Kroah-Hartman wrote: On Thu, Aug 25, 2022 at 01:36:10AM +0200, Alejandro Colomar wrote: But from your side what do we have? Just direct NAKs without much explanation. The only one who gave some explanation was Greg, and he vaguely pointed to Linus's comments about it in the past, with no precise pointer to it. I investigated a lot before v2, and could not find anything strong enough to recommend using kernel types in user space, so I pushed v2, and the discussion was kept. So despite me saying that "this is not ok", and many other maintainers saying "this is not ok", you applied a patch with our objections on it? That is very odd and a bit rude. I would like that if you still oppose to the patch, at least were able to provide some facts to this discussion. The fact is that the kernel can not use the namespace that userspace has with ISO C names. It's that simple as the ISO standard does NOT describe the variable types for an ABI that can cross the user/kernel boundry. I understand that. But user-space programs are allowed to use the standard types when calling a syscall that really uses kernel types. IMHO, it should be irrelevant for the user how the kernel decides to call a 64-bit unsigned integer, right? Or do you mean that some of the pages I modified ... are intended mostly for kernel-space programmers? Work with the ISO C standard if you wish to document such type usage, and get it approved and then we would be willing to consider such a change. But until then, we have to stick to our variable name types, just like all other operating systems have to (we are not alone here.) Please revert your change. Thanks for asking nicely. Since there's ongoing discussion, and I don't want to make it look like ignoring it, I've reverted the patch for now. If I apply it again, I hope that it will be with some more consensus, as I've always tried to do. Sorry if I was a bit irascible yesterday. Shit happens. TL;DR: Patch reverted; asking nicely works. =) greg k-h Cheers, Alex -- Alejandro Colomar <http://www.alejandro-colomar.es/> OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming
Hi Linus,
(Oops, I mistyped you name in my previous reply; I'm on a roll for funny
typos this week it seems)
On 8/25/22 09:42, Linus Torvalds wrote:
On Thu, Aug 25, 2022 at 12:20 AM Alejandro Colomar
wrote:
This patch is not about kernel, but about the section 2 and 3 manual
pages, which are directed towards user-space readers most of the time.
They are about the types to the kernel interfaces. Those types that
the kernel defines and exposes.
And the kernel type in question looks like this:
struct { /* anonymous struct used by BPF_PROG_LOAD command */
__u32 prog_type; /* one of enum bpf_prog_type */
__u32 insn_cnt;
__aligned_u64 insns;
__aligned_u64 license;
because the kernel UAPI header wants to
(a) work whether or not was included
These days, (a) is more of a theoretical thing, since programs avoiding
C99 will have a hard time.
(b) doesn't want to include so as to not pollute the namespace
(c) actually wants to use our special types
I quoted a few more fields for that (c) reason: we've had a long
history of getting the user space API wrong due to strange alignment
issues, where 32-bit and 64-bit targets had different alignment for
types. So then we ended up with various compat structures to translate
from one to the other because they had all the same fields, but
different padding.
This happened a few times with the DRM people, and they finally got
tired of it, and started using that "__aligned_u64" type, which is
just the same old __u64, but explicitly aligned to its natural
alignment.
So these are the members that the interface actually uses.
If you document those members, wouldn't it be good to have that
documentation be actually accurate?
Honestly, I don't think it makes a *huge* amount of difference, but
documentation that doesn't actually match the source of the
documentation will just confuse somebody in the end. Somebody will go
"that's not right", and maybe even change the structure definitions to
match the documentation.
Which would be wrong.
Now, you don't have to take the kernel uapi headers. We *try* to make
those usable as-is, but hey, there has been problems in the past, and
it's not necessarily wrong to take the kernel header and then munge it
to be "appropriate" for user space.
So I guess you can just make your own structures with the names and
syntax you want, and say "these are *my* header files, and *my*
documentation for them".
That's fine. But I'm not surprised if the kernel maintainer then goes
"no, that's not the interface I agreed to" if only because it's a pain
to verify that you got it all right. Maybe it was all trivial and
automated and there can be no errors, but it's still a "why is there a
different copy of this complicated interface".
If you really want to describe things to people, wouldn't it be nicer
to just say "there's a 32-bit unsigned 'prog_type' member" and leave
it at that?
Do you really want to enforce your opinion of what is prettier on the
maintainer that wrote that thing, and then argue with him when he
doesn't agree?
You convinced me. The man-pages will document the types exactly as they
are in kernel. It's just simpler.
As the patch was recently reverted after Greg asked me to do, I'll keep
it that way. I guess this closes the man-pages discussion.
I'd still like to see the kernel types be API-compatible with the
user-space ones, for which there's a patch around, and also making the
types be builtind could also be nice. Let's see if that
works out.
Cheers,
Alex
--
Alejandro Colomar
<http://www.alejandro-colomar.es/>
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming
Hi Xi, On 8/25/22 09:28, Xi Ruoyao wrote: On Thu, 2022-08-25 at 09:20 +0200, Alejandro Colomar via Gcc-patches wrote: I don't know for sure, and I never pretended to say otherwise. But what IMHO the kernel could do is to make the types compatible, by typedefing to the same fundamental types (i.e., long or long long) that user-space types do. In user-space things are already inconsistent as we have multiple libc implementations. Telling every libc implementation to sync their typedef w/o a WG14 decision will only cause "aggressive discussion" (far more aggressive than this thread, I'd say). If int64_t etc. were defined as builtin types since epoch, things would be a lot easier. But we can't change history. This would be great. I mean, the fundamental types should be u8, u16, ... and int, long, ... typedefs for these, and not the other way around, if the language was designed today. Maybe GCC could consider something like that. Cheers, Alex -- Alejandro Colomar <http://www.alejandro-colomar.es/> OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming
Hi Greg, On 8/25/22 07:57, Greg Kroah-Hartman wrote: On Thu, Aug 25, 2022 at 01:36:10AM +0200, Alejandro Colomar wrote: But from your side what do we have? Just direct NAKs without much explanation. The only one who gave some explanation was Greg, and he vaguely pointed to Linus's comments about it in the past, with no precise pointer to it. I investigated a lot before v2, and could not find anything strong enough to recommend using kernel types in user space, so I pushed v2, and the discussion was kept. So despite me saying that "this is not ok", and many other maintainers saying "this is not ok", you applied a patch with our objections on it? That is very odd and a bit rude. I would like that if you still oppose to the patch, at least were able to provide some facts to this discussion. The fact is that the kernel can not use the namespace that userspace has with ISO C names. It's that simple as the ISO standard does NOT describe the variable types for an ABI that can cross the user/kernel boundry. I understand that. But user-space programs are allowed to use the standard types when calling a syscall that really uses kernel types. IMHO, it should be irrelevant for the user how the kernel decides to call a 64-bit unsigned integer, right? Or do you mean that some of the pages I modified Work with the ISO C standard if you wish to document such type usage, and get it approved and then we would be willing to consider such a change. But until then, we have to stick to our variable name types, just like all other operating systems have to (we are not alone here.) Please revert your change. Thanks for asking nicely. Since there's ongoing discussion, and I don't want to make it look like ignoring it, I've reverted the patch for now. If I apply it again, I hope that it will be with some more consensus, as I've always tried to do. Sorry if I was a bit irascible yesterday. Shit happens. TL;DR: Patch reverted; asking nicely works. =) greg k-h Cheers, Alex -- Alejandro Colomar <http://www.alejandro-colomar.es/> OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming
Hi Linux, On 8/25/22 02:52, Linus Torvalds wrote: On Wed, Aug 24, 2022 at 4:36 PM Alejandro Colomar wrote: I'm trying to be nice, and ask for review to make sure I'm not making some big mistake by accident, and I get disrespect? No thanks. You've been told multiple times that the kernel doesn't use the "standard" names, and *cannot* use them for namespace reasons, and you ignore all the feedback, and then you claim you are asking for review? This patch is not about kernel, but about the section 2 and 3 manual pages, which are directed towards user-space readers most of the time. Admittedly, some syscalls are only callable from within the kernel itself, but that's very rare. [...] The fact is, kernel UAPI header files MUST NOT use the so-called standard names. I don't know for sure, and I never pretended to say otherwise. But what IMHO the kernel could do is to make the types compatible, by typedefing to the same fundamental types (i.e., long or long long) that user-space types do. We cannot provide said names, because they are only provided by the standard header files. And since kernel header files cannot provide them, then kernel UAPI header files cannot _use_ them. End result: any kernel UAPI header file will continue to use __u32 etc naming that doesn't have any namespace pollution issues. Nothing else is even remotely acceptable. Stop trying to make this something other than it is. And if you cannot accept these simple technical reasons, why do you expect respect? Why are you so special that you think you can change the rules for kernel uapi files over the *repeated* objections from maintainers who know better? No sorry, if someone understood this patch as changing anything in UAPI, it is not. Cheers, Alex -- Alejandro Colomar <http://www.alejandro-colomar.es/> OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3] Many pages: Document fixed-width types with ISO C naming
Alexei,
On 8/24/22 20:55, Alejandro Colomar wrote:
> Link:
<https://lore.kernel.org/linux-man/20210423230609.13519-1-alx.manpa...@gmail.com/T/>
> Link: <https://lore.kernel.org/lkml/YZvIlz7J6vOEY+Xu@yuki/T/>
> Signed-off-by: Alejandro Colomar
> Nacked-by: Alexei Starovoitov
> Nacked-by: Greg Kroah-Hartman
> Nacked-by: Daniel Borkmann
> Acked-by: Zack Weinberg
On 8/25/22 00:40, Alexei Starovoitov wrote:
On Wed, Aug 24, 2022 at 12:04 PM Alejandro Colomar
wrote:
diff --git a/man2/bpf.2 b/man2/bpf.2
index d05b73ec2..84d1b62e5 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
[...]
struct {/* Used by BPF_PROG_LOAD */
-__u32 prog_type;
-__u32 insn_cnt;
+uint32_t prog_type;
+uint32_t insn_cnt;
For the N-th time:
Nacked-by: Alexei Starovoitov
Please stop sending this patch.
Sorry, but no.
First, this has only been v3, and v1 was a year and a half ago, don't
make it like I'm constantly making you lose your precious time, because
I'm actively trying not to.
Second, I already made a big notice that you and a few more have
strongly opposed to the patch, respectfully keeping all of your NAKs in
my patch, as you can see above.
I gave very detailed reasons of why this patch is reasonable and, back
in the days of v1, Zack (from glibc) gave even better reasons of why the
manual pages should document ISO C (libc) types, and not kernel ones,
and why it shouldn't matter to user-space programmers.
But from your side what do we have? Just direct NAKs without much
explanation. The only one who gave some explanation was Greg, and he
vaguely pointed to Linus's comments about it in the past, with no
precise pointer to it. I investigated a lot before v2, and could not
find anything strong enough to recommend using kernel types in user
space, so I pushed v2, and the discussion was kept.
I would like that if you still oppose to the patch, at least were able
to provide some facts to this discussion.
But the most fundamental thing that I ask is that you respect me.
With this attitude, the only thing you're going to get is that I apply
the patch right after, because:
1) The patch is right. Go talk to glibc and gcc maintainers, who know
how types work by heart if you have doubts.
2) I'm the maintainer of this project, and under doubts, it's my decission.
I'm trying to be nice, and ask for review to make sure I'm not making
some big mistake by accident, and I get disrespect? No thanks.
Patch applied.
Now, if someone with a bit more respect still thinks this change is
incorrect, and is wanting to share some facts to show me my mistake,
I'll happily review it and revert the patch if necessary. For now, the
patch is applied.
Alex
--
Alejandro Colomar
Linux man-pages maintainer
<http://www.alejandro-colomar.es/>
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH v3] Many pages: Document fixed-width types with ISO C naming
Kernel __u64 and similar types are ABI-compatible, and mostly
API-compatible with ISO C types. User-space programmers don't
care about kernel details, and should only use libc types.
Document syscalls and structures provided by the Linux kernel as
if they used libc types.
There's work in the kernel to remove this small API
incompatibility, which is only for pointers or printf specifiers.
Since I couldn't find any structure that uses pointers, there
shouldn't be any issues here. Also, the only pointer I found was
in a syscall parameter, but since syscall(2) doesn't check its
arguments' types, we're also safe there.
This patch doesn't go without controversy. Check the discussions
in the links below.
Found with:
$ grep -rn '\b_*[su][8136][624]*\b' man* \
| grep -v -e /bpf-helpers.7 -e /proc.5 -e /epoll_event.3type -e /wcscmp.3 \
-e /crypt.3 -e /mempcpy.3 -e /memcmp.3 -e /string.3 -e /wcsncmp.3 \
-e /wcscasecmp.3 -e /wmemcmp.3 -e /strcasecmp.3 -e /bcmp.3 \
-e /bstring.3 -e /endian.3 -e /strverscmp.3 -e /wcsncasecmp.3 \
-e /strcoll.3 -e /strcmp.3 \
| tee /dev/tty \
| wc -l;
Link:
<https://lore.kernel.org/linux-man/20210423230609.13519-1-alx.manpa...@gmail.com/T/>
Link: <https://lore.kernel.org/lkml/YZvIlz7J6vOEY+Xu@yuki/T/>
Signed-off-by: Alejandro Colomar
Nacked-by: Alexei Starovoitov
Nacked-by: Greg Kroah-Hartman
Nacked-by: Daniel Borkmann
Acked-by: Zack Weinberg
Cc: LKML
Cc: glibc
Cc: GCC
Cc: bpf
Cc: LTP List
Cc: Linux API
Cc: linux-arch
Cc: David Laight
Cc: Joseph Myers
Cc: Florian Weimer
Cc: Daniel Borkmann
Cc: Cyril Hrubis
Cc: David Howells
Cc: Arnd Bergmann
Cc: Florian Weimer
Cc: Rich Felker
Cc: Adhemerval Zanella
---
Hi,
I removed attributes from this patch. It now only changes types,
and only types that don't have anything special in them. __be64
or things like that are not included in this patch. __aligned_u64
isn't either.
I made clear what the incompatibilities are, and I hope Cyril
fixes them soon, but anyway, they shouldn't affect for the cases
documented here. And if they affect somehow, a programmer will
probably know the reason; but I'd consider that a kernel bug, or
at least something to improve there.
I'm decided to push for this change, and would like to see the
kernel making the necessary steps to make it smoother, since it
shouldn't be hard (the patch is there already; it only needs a
little bit of love).
Anyway, I don't think it will cause important problems to readers
of the manual pages, and instead expect an improvement.
Cheers,
Alex
man2/bpf.2 | 28 +-
man2/capget.2 | 10 +-
man2/clone.2 | 36 +--
man2/getunwind.2 | 6 +-
man2/io_submit.2 | 22 +-
man2/ioctl_ficlonerange.2 | 8 +-
man2/ioctl_fideduperange.2 | 20 +-
man2/ioctl_getfsmap.2 | 28 +-
man2/ioctl_userfaultfd.2 | 37 +--
man2/kcmp.2| 6 +-
man2/keyctl.2 | 8 +-
man2/landlock_add_rule.2 | 4 +-
man2/landlock_create_ruleset.2 | 2 +-
man2/mount_setattr.2 | 8 +-
man2/perf_event_open.2 | 486 -
man2/ptrace.2 | 40 +--
man2/sched_setattr.2 | 20 +-
man2/seccomp.2 | 24 +-
man2/seccomp_unotify.2 | 32 +--
man2/statx.2 | 42 +--
man2/userfaultfd.2 | 28 +-
man2type/open_how.2type| 6 +-
man3type/epoll_event.3type | 2 +-
man4/loop.4| 6 +-
man4/random.4 | 6 +-
man7/fanotify.7| 28 +-
man7/netdevice.7 | 2 +-
man7/netlink.7 | 14 +-
man7/packet.7 | 16 +-
man7/rtnetlink.7 | 20 +-
man7/sock_diag.7 | 122 +
31 files changed, 568 insertions(+), 549 deletions(-)
diff --git a/man2/bpf.2 b/man2/bpf.2
index d05b73ec2..84d1b62e5 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -169,34 +169,34 @@ commands:
.EX
union bpf_attr {
struct {/* Used by BPF_MAP_CREATE */
-__u32 map_type;
-__u32 key_size;/* size of key in bytes */
-__u32 value_size; /* size of value in bytes */
-__u32 max_entries; /* maximum number of entries
+uint32_t map_type;
+uint32_t key_size;/* size of key in bytes */
+uint32_t value_size; /* size of value in bytes */
+uint32_t max_entries; /* maximum number of entries
in a map */
};
struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
commands */
-__u32 map_fd;
+uint32_t map_fd;
__aligned_u64 key;
union {
__aligned_u64 value;
__aligned_u64 next_key;
};
-
Re: [PATCH v3] bpf.2: Use standard types and attributes
Hello Daniel, On 5/17/21 8:56 PM, Daniel Borkmann wrote: On 5/16/21 11:16 AM, Alejandro Colomar (man-pages) wrote: Signed-off-by: Alejandro Colomar Discussion: <https://lore.kernel.org/linux-man/6740a229-842e-b368-86eb-defc786b3...@gmail.com/T/> Nacked-by: Alexei Starovoitov Nacked-by: Greg Kroah-Hartman You forgot to retain my ... Nacked-by: Daniel Borkmann Yup! Sorry, I forgot :) Thanks, Alex Acked-by: Zack Weinberg Cc: LKML Cc: glibc Cc: GCC Cc: bpf Cc: David Laight Cc: Joseph Myers Cc: Florian Weimer Cc: Daniel Borkmann --- man2/bpf.2 | 49 - 1 file changed, 24 insertions(+), 25 deletions(-)
Re: [PATCH v3] bpf.2: Use standard types and attributes
On 5/15/21 9:01 PM, Alejandro Colomar wrote:
Some manual pages are already using C99 syntax for integral
types 'uint32_t', but some aren't. There are some using kernel
syntax '__u32'. Fix those.
Both the kernel and the standard types are 100% binary compatible,
and the source code differences between them are very small, and
not important in a manual page:
- Some of them are implemented with different underlying types
(e.g., s64 is always long long, while int64_t may be long long
or long, depending on the arch). This causes the following
differences.
- length modifiers required by printf are different, resulting in
a warning ('-Wformat=').
- pointer assignment causes a warning:
('-Wincompatible-pointer-types'), but there aren't any pointers
in this page.
But, AFAIK, all of those warnings can be safely ignored, due to
the binary compatibility between the types.
...
Some pages also document attributes, using GNU syntax
'__attribute__((xxx))'. Update those to use the shorter and more
portable C11 keywords such as 'alignas()' when possible, and C2x
syntax '[[gnu::xxx]]' elsewhere, which hasn't been standardized
yet, but is already implemented in GCC, and available through
either --std=c2x or any of the --std=gnu... options.
The standard isn't very clear on how to use alignas() or
[[]]-style attributes, and the GNU documentation isn't better, so
the following link is a useful experiment about the different
alignment syntaxes:
__attribute__((aligned())), alignas(), and [[gnu::aligned()]]:
<https://stackoverflow.com/q/67271825/6872717>
Signed-off-by: Alejandro Colomar
Discussion:
<https://lore.kernel.org/linux-man/6740a229-842e-b368-86eb-defc786b3...@gmail.com/T/>
Nacked-by: Alexei Starovoitov
Nacked-by: Greg Kroah-Hartman
Acked-by: Zack Weinberg
Cc: LKML
Cc: glibc
Cc: GCC
Cc: bpf
Cc: David Laight
Cc: Joseph Myers
Cc: Florian Weimer
Cc: Daniel Borkmann
---
man2/bpf.2 | 49 -
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/man2/bpf.2 b/man2/bpf.2
index 6e1ffa198..04b8fbcef 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -186,41 +186,40 @@ commands:
.PP
.in +4n
.EX
-union bpf_attr {
+union [[gnu::aligned(8)]] bpf_attr {
struct {/* Used by BPF_MAP_CREATE */
-__u32 map_type;
-__u32 key_size;/* size of key in bytes */
-__u32 value_size; /* size of value in bytes */
-__u32 max_entries; /* maximum number of entries
- in a map */
+uint32_tmap_type;
+uint32_tkey_size;/* size of key in bytes */
+uint32_tvalue_size; /* size of value in bytes */
+uint32_tmax_entries; /* maximum number of entries
+in a map */
};
-struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
- commands */
-__u32 map_fd;
-__aligned_u64 key;
+struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY commands */
+uint32_tmap_fd;
+uint64_t alignas(8) key;
union {
-__aligned_u64 value;
-__aligned_u64 next_key;
+uint64_t alignas(8) value;
+uint64_t alignas(8) next_key;
};
-__u64 flags;
+uint64_tflags;
};
struct {/* Used by BPF_PROG_LOAD */
-__u32 prog_type;
-__u32 insn_cnt;
-__aligned_u64 insns; /* \(aqconst struct bpf_insn *\(aq */
-__aligned_u64 license;/* \(aqconst char *\(aq */
-__u32 log_level; /* verbosity level of verifier */
-__u32 log_size; /* size of user buffer */
-__aligned_u64 log_buf;/* user supplied \(aqchar *\(aq
- buffer */
-__u32 kern_version;
- /* checked when prog_type=kprobe
- (since Linux 4.1) */
+uint32_tprog_type;
+uint32_tinsn_cnt;
+uint64_t alignas(8) insns; /* \(aqconst struct bpf_insn *\(aq */
+uint64_t alignas(8) license; /* \(aqconst char *\(aq */
+uint32_tlog_level; /* verbosity level of verifier */
+uint32_tlog_size; /* size of user buffer */
+uint64_t alignas(8) log_buf; /* user supplied \(aqchar *\(aq
+ buffer */
+uint32_tkern_version;
+ /* checked when prog_type=kprobe
+ (since Linux 4.1) */
.\" commit 2541517c32be2531e0da59dfd7efc1ce844644f5
};
-} __attribute__((aligned(8)));
+};
.EE
.in
.\"
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.ke
[PATCH v3] bpf.2: Use standard types and attributes
Some manual pages are already using C99 syntax for integral
types 'uint32_t', but some aren't. There are some using kernel
syntax '__u32'. Fix those.
Both the kernel and the standard types are 100% binary compatible,
and the source code differences between them are very small, and
not important in a manual page:
- Some of them are implemented with different underlying types
(e.g., s64 is always long long, while int64_t may be long long
or long, depending on the arch). This causes the following
differences.
- length modifiers required by printf are different, resulting in
a warning ('-Wformat=').
- pointer assignment causes a warning:
('-Wincompatible-pointer-types'), but there aren't any pointers
in this page.
But, AFAIK, all of those warnings can be safely ignored, due to
the binary compatibility between the types.
...
Some pages also document attributes, using GNU syntax
'__attribute__((xxx))'. Update those to use the shorter and more
portable C11 keywords such as 'alignas()' when possible, and C2x
syntax '[[gnu::xxx]]' elsewhere, which hasn't been standardized
yet, but is already implemented in GCC, and available through
either --std=c2x or any of the --std=gnu... options.
The standard isn't very clear on how to use alignas() or
[[]]-style attributes, and the GNU documentation isn't better, so
the following link is a useful experiment about the different
alignment syntaxes:
__attribute__((aligned())), alignas(), and [[gnu::aligned()]]:
<https://stackoverflow.com/q/67271825/6872717>
Signed-off-by: Alejandro Colomar
Nacked-by: Alexei Starovoitov
Nacked-by: Greg Kroah-Hartman
Acked-by: Zack Weinberg
Cc: LKML
Cc: glibc
Cc: GCC
Cc: bpf
Cc: David Laight
Cc: Joseph Myers
Cc: Florian Weimer
Cc: Daniel Borkmann
---
man2/bpf.2 | 49 -
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/man2/bpf.2 b/man2/bpf.2
index 6e1ffa198..04b8fbcef 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -186,41 +186,40 @@ commands:
.PP
.in +4n
.EX
-union bpf_attr {
+union [[gnu::aligned(8)]] bpf_attr {
struct {/* Used by BPF_MAP_CREATE */
-__u32 map_type;
-__u32 key_size;/* size of key in bytes */
-__u32 value_size; /* size of value in bytes */
-__u32 max_entries; /* maximum number of entries
- in a map */
+uint32_tmap_type;
+uint32_tkey_size;/* size of key in bytes */
+uint32_tvalue_size; /* size of value in bytes */
+uint32_tmax_entries; /* maximum number of entries
+in a map */
};
-struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
- commands */
-__u32 map_fd;
-__aligned_u64 key;
+struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY commands */
+uint32_tmap_fd;
+uint64_t alignas(8) key;
union {
-__aligned_u64 value;
-__aligned_u64 next_key;
+uint64_t alignas(8) value;
+uint64_t alignas(8) next_key;
};
-__u64 flags;
+uint64_tflags;
};
struct {/* Used by BPF_PROG_LOAD */
-__u32 prog_type;
-__u32 insn_cnt;
-__aligned_u64 insns; /* \(aqconst struct bpf_insn *\(aq */
-__aligned_u64 license;/* \(aqconst char *\(aq */
-__u32 log_level; /* verbosity level of verifier */
-__u32 log_size; /* size of user buffer */
-__aligned_u64 log_buf;/* user supplied \(aqchar *\(aq
- buffer */
-__u32 kern_version;
- /* checked when prog_type=kprobe
- (since Linux 4.1) */
+uint32_tprog_type;
+uint32_tinsn_cnt;
+uint64_t alignas(8) insns; /* \(aqconst struct bpf_insn *\(aq */
+uint64_t alignas(8) license; /* \(aqconst char *\(aq */
+uint32_tlog_level; /* verbosity level of verifier */
+uint32_tlog_size; /* size of user buffer */
+uint64_t alignas(8) log_buf; /* user supplied \(aqchar *\(aq
+ buffer */
+uint32_tkern_version;
+ /* checked when prog_type=kprobe
+ (since Linux 4.1) */
.\" commit 2541517c32be2531e0da59dfd7efc1ce844644f5
};
-} __attribute__((aligned(8)));
+};
.EE
.in
.\"
--
2.31.1
Re: [RFC v2] bpf.2: Use standard types and attributes
Hi Daniel,
On 5/4/21 10:06 PM, Daniel Borkmann wrote:
On 5/4/21 6:08 PM, Daniel Borkmann wrote:
>
> But what /problem/ is this really solving? Why bother to change
this /now/
> after so many years?! I think this is causing more confusion than
solving
> anything, really. Moreover, what are you doing with all the
> __{le,be}{16,32,64}
> types in uapi? Anyway, NAK for bpf.2 specifically, and the idea
generally..
I'm trying to clarify the manual pages as much as possible, by using
standard conventions and similar structure all around the pages. Not
everyone understands kernel conventions. Basically, Zack said very
much what I had in mind with this patch.
But then are you also converting, for example, __{le,be}{16,32,64} to plain
uint{16,32,64}_t in the man pages and thus removing contextual information
(or inventing new equivalent types)?
What about other types exposed to user space like __sum16, __wsum, or
__poll_t
when they are part of a man page, etc?
Sorry, I forgot to address that part in my answer. If there's no
standard way of naming a type without losing information, we can use the
kernel naming. I have no objection to that.
These are the only pages that seem to be using those:
$ grep -Enr '\b__[a-z][a-z]+[0-9]+' man?
man2/clone.2:44:clone, __clone2, clone3 \- create a child process
man2/clone.2:1694:.BI "int __clone2(int (*" "fn" ")(void *),"
man2/clone.2:1717:.BR __clone2 ()
man7/sock_diag.7:362:__be16 idiag_sport;
man7/sock_diag.7:363:__be16 idiag_dport;
man7/sock_diag.7:364:__be32 idiag_src[4];
man7/sock_diag.7:365:__be32 idiag_dst[4];
man7/bpf-helpers.7:514:.B \fBlong bpf_skb_vlan_push(struct sk_buff
*\fP\fIskb\fP\fB, __be16\fP \fIvlan_proto\fP\fB, u16\fP
\fIvlan_tci\fP\fB)\fP
man7/bpf-helpers.7:878:.B \fBs64 bpf_csum_diff(__be32 *\fP\fIfrom\fP\fB,
u32\fP \fIfrom_size\fP\fB, __be32 *\fP\fIto\fP\fB, u32\fP
\fIto_size\fP\fB, __wsum\fP \fIseed\fP\fB)\fP
man7/bpf-helpers.7:949:.B \fBlong bpf_skb_change_proto(struct sk_buff
*\fP\fIskb\fP\fB, __be16\fP \fIproto\fP\fB, u64\fP \fIflags\fP\fB)\fP
man7/system_data_types.7:473:.I __int128
man7/system_data_types.7:475:.I __int128
man7/system_data_types.7:1584:.I unsigned __int128
man7/system_data_types.7:1586:.I unsigned __int128
$
So sock_diag.7 and bpf-helpers.7 and only a handful of cases. Not much
of a problem. I'd keep those untouched.
Regards,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
Re: [RFC v2] bpf.2: Use standard types and attributes
Hi Florian, On 5/4/21 9:45 PM, Florian Weimer wrote: * Alejandro Colomar: The thing is, in all of those threads, the only reasons to avoid types in the kernel (at least, the only explicitly mentioned ones) are (a bit simplified, but this is the general idea of those threads): * Possibly breaking something in such a big automated change. * Namespace collision with userspace (the C standard allows defining uint32_t for nefarious purposes as long as you don't include . POSIX prohibits that, though) * Uglier __u64 can't be formatted with %llu on all architectures. That's not true for uint64_t, where you have to use %lu on some architectures to avoid compiler warnings (and technically undefined behavior). There are preprocessor macros to get the expected format specifiers, but they are clunky. I don't know if the problem applies to uint32_t. It does happen with size_t and ptrdiff_t on 32-bit targets (both vary between int and long). Hmmm, that's interesting. It looks like Linux always uses long long for 64 bit types, while glibc uses 'long' as long as it's possible, and only uses 'long long' when necessary. Assignment is still 100% valid both ways and binary compatibility also 100% (AFAIK), given they're the same length and signedness, but pointers are incompatible. That's something to note, even though in this case there are no pointers involved, so no incompatibilities. Maybe the kernel and glibc could use the same rules to improve compatibility, but that's out of the scope of this. Thanks, Alex -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
Re: [RFC v2] bpf.2: Use standard types and attributes
tions. Basically, Zack said very much what I had in mind with this patch. Thanks for your reviews! Regards, Alex -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
Re: [RFC v2] bpf.2: Use standard types and attributes
Hi Greg and Alexei,
On Tue, May 04, 2021 at 07:12:01AM -0700, Alexei Starovoitov wrote:
For the same reasons as explained earlier:
Nacked-by: Alexei Starovoitov
Okay, I'll add that.
On 5/4/21 4:24 PM, Greg KH wrote:> I agree, the two are not the same
type at all, this change should not be
accepted.
I get that in the kernel you don't use the standard fixed-width types
(with some exceptions), probably not to mess with code that relies on
not being included (I hope there's not much code that relies
on this in 2021, but who knows).
But, there is zero difference between these types, from the point of
view of the compiler. There's 100% compatibility between those types,
and you're able to mix'n'match them. See some example below.
Could you please explain why the documentation, which supposedly only
documents the API and not the internal implementation, should not use
standard naming conventions? The standard is much easier to read for
userspace programmers, which might ignore why the kernel does some
things in some specific ways.
BTW, just to clarify, bpf.2 is just a small sample to get reviews; the
original intention was to replace __uNN by uintNN_t in all of the manual
pages.
Thanks,
Alex
...
Example:
$ cat test.c
#include
typedef int __s32;
int32_t foo(void);
int main(void)
{
return 1 - foo();
}
__s32 foo(void)
{
return 1;
}
$ cc -Wall -Wextra -Werror -S -Og test.c -o test.s
$ cat test.s
.file "test.c"
.text
.globl foo
.type foo, @function
foo:
.LFB1:
.cfi_startproc
movl$1, %eax
ret
.cfi_endproc
.LFE1:
.size foo, .-foo
.globl main
.type main, @function
main:
.LFB0:
.cfi_startproc
callfoo
movl%eax, %edx
movl$1, %eax
subl%edx, %eax
ret
.cfi_endproc
.LFE0:
.size main, .-main
.ident "GCC: (Debian 10.2.1-6) 10.2.1 20210110"
.section.note.GNU-stack,"",@progbits
$
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
[RFC v2] bpf.2: Use standard types and attributes
Some manual pages are already using C99 syntax for integral
types 'uint32_t', but some aren't. There are some using kernel
syntax '__u32'. Fix those.
Some pages also document attributes, using GNU syntax
'__attribute__((xxx))'. Update those to use the shorter and more
portable C11 keywords such as 'alignas()' when possible, and C2x
syntax '[[gnu::xxx]]' elsewhere, which hasn't been standardized
yet, but is already implemented in GCC, and available through
either --std=c2x or any of the --std=gnu... options.
The standard isn't very clear on how to use alignas() or
[[]]-style attributes, so the following link is useful in the case
of 'alignas()' and '[[gnu::aligned()]]':
<https://stackoverflow.com/q/67271825/6872717>
Signed-off-by: Alejandro Colomar
Cc: LKML
Cc: glibc
Cc: GCC
Cc: Alexei Starovoitov
Cc: bpf
Cc: David Laight
Cc: Zack Weinberg
Cc: Joseph Myers
---
man2/bpf.2 | 49 -
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/man2/bpf.2 b/man2/bpf.2
index 6e1ffa198..04b8fbcef 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -186,41 +186,40 @@ commands:
.PP
.in +4n
.EX
-union bpf_attr {
+union [[gnu::aligned(8)]] bpf_attr {
struct {/* Used by BPF_MAP_CREATE */
-__u32 map_type;
-__u32 key_size;/* size of key in bytes */
-__u32 value_size; /* size of value in bytes */
-__u32 max_entries; /* maximum number of entries
- in a map */
+uint32_tmap_type;
+uint32_tkey_size;/* size of key in bytes */
+uint32_tvalue_size; /* size of value in bytes */
+uint32_tmax_entries; /* maximum number of entries
+in a map */
};
-struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
- commands */
-__u32 map_fd;
-__aligned_u64 key;
+struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY commands */
+uint32_tmap_fd;
+uint64_t alignas(8) key;
union {
-__aligned_u64 value;
-__aligned_u64 next_key;
+uint64_t alignas(8) value;
+uint64_t alignas(8) next_key;
};
-__u64 flags;
+uint64_tflags;
};
struct {/* Used by BPF_PROG_LOAD */
-__u32 prog_type;
-__u32 insn_cnt;
-__aligned_u64 insns; /* \(aqconst struct bpf_insn *\(aq */
-__aligned_u64 license;/* \(aqconst char *\(aq */
-__u32 log_level; /* verbosity level of verifier */
-__u32 log_size; /* size of user buffer */
-__aligned_u64 log_buf;/* user supplied \(aqchar *\(aq
- buffer */
-__u32 kern_version;
- /* checked when prog_type=kprobe
- (since Linux 4.1) */
+uint32_tprog_type;
+uint32_tinsn_cnt;
+uint64_t alignas(8) insns; /* \(aqconst struct bpf_insn *\(aq */
+uint64_t alignas(8) license; /* \(aqconst char *\(aq */
+uint32_tlog_level; /* verbosity level of verifier */
+uint32_tlog_size; /* size of user buffer */
+uint64_t alignas(8) log_buf; /* user supplied \(aqchar *\(aq
+ buffer */
+uint32_tkern_version;
+ /* checked when prog_type=kprobe
+ (since Linux 4.1) */
.\" commit 2541517c32be2531e0da59dfd7efc1ce844644f5
};
-} __attribute__((aligned(8)));
+};
.EE
.in
.\"
--
2.31.1
Re: [RFC] bpf.2: Use standard types and attributes
Hi Joseph, On 4/26/21 7:19 PM, Joseph Myers wrote: On Sat, 24 Apr 2021, Alejandro Colomar via Libc-alpha wrote: Some pages also document attributes, using GNU syntax '__attribute__((xxx))'. Update those to use the shorter and more portable C2x syntax, which hasn't been standardized yet, but is already implemented in GCC, and available through either --std=c2x or any of the --std=gnu... options. If you mention alignment in the manpage at all, the same reasoning would say you should use _Alignas(8) not [[gnu::aligned(8)]], in any context where _Alignas is valid. Agree. I just didn't know 'alignas()' (a.k.a. '_Alignas()'), so I used attributes and only changed the syntax. But yes, we should use that C11 feature. Given that we already used 'noreturn' and not '_Noreturn' (see exit(3) and its family), I'll use 'alignas()'. I'll send a v2 with those changes. Thanks, Alex -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
Re: [RFC] bpf.2: Use standard types and attributes
Hello Alexei, On 4/24/21 1:20 AM, Alexei Starovoitov wrote: Nack. The man page should describe the kernel api the way it is in .h file. Why? When glibc uses __size_t (or any other non-standard types) just because the standard doesn't allow it to define some types in some specific header, the manual pages document the equivalent standard type, (i.e., if glibc uses __size_t, we document size_t). The compiler, AFAIK (gcc is CCd, so they can jump in if I'm wrong), using uint32_t in every situation where __u32 is expected. They're both typedefs for the same basic type. I can understand why Linux will keep using u32 types (and their __ user space variants), but that doesn't mean user space programs need to use the same type. If we have a standard syntax for fixed-width integral types (and for anything, actually), the manual pages should probably follow it, whenever possible. Any deviation from the standard (be it C or POSIX) should have a very good reason to be; otherwise, it only creates confusion. Thanks, Alex -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ Senior SW Engineer; http://www.alejandro-colomar.es/
[RFC] bpf.2: Use standard types and attributes
Some manual pages are already using C99 syntax for integral
types 'uint32_t', but some aren't. There are some using kernel
syntax '__u32'. Fix those.
Some pages also document attributes, using GNU syntax
'__attribute__((xxx))'. Update those to use the shorter and more
portable C2x syntax, which hasn't been standardized yet, but is
already implemented in GCC, and available through either --std=c2x
or any of the --std=gnu... options.
Signed-off-by: Alejandro Colomar
---
man2/bpf.2 | 47 +++
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/man2/bpf.2 b/man2/bpf.2
index 6e1ffa198..204f01bfc 100644
--- a/man2/bpf.2
+++ b/man2/bpf.2
@@ -188,39 +188,38 @@ commands:
.EX
union bpf_attr {
struct {/* Used by BPF_MAP_CREATE */
-__u32 map_type;
-__u32 key_size;/* size of key in bytes */
-__u32 value_size; /* size of value in bytes */
-__u32 max_entries; /* maximum number of entries
- in a map */
+uint32_tmap_type;
+uint32_tkey_size;/* size of key in bytes */
+uint32_tvalue_size; /* size of value in bytes */
+uint32_tmax_entries; /* maximum number of entries
+in a map */
};
-struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY
- commands */
-__u32 map_fd;
-__aligned_u64 key;
+struct {/* Used by BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY commands */
+uint32_t map_fd;
+uint64_t [[gnu::aligned(8)]] key;
union {
-__aligned_u64 value;
-__aligned_u64 next_key;
+uint64_t [[gnu::aligned(8)]] value;
+uint64_t [[gnu::aligned(8)]] next_key;
};
-__u64 flags;
+uint64_t flags;
};
struct {/* Used by BPF_PROG_LOAD */
-__u32 prog_type;
-__u32 insn_cnt;
-__aligned_u64 insns; /* \(aqconst struct bpf_insn *\(aq */
-__aligned_u64 license;/* \(aqconst char *\(aq */
-__u32 log_level; /* verbosity level of verifier */
-__u32 log_size; /* size of user buffer */
-__aligned_u64 log_buf;/* user supplied \(aqchar *\(aq
- buffer */
-__u32 kern_version;
- /* checked when prog_type=kprobe
- (since Linux 4.1) */
+uint32_t prog_type;
+uint32_t insn_cnt;
+uint64_t [[gnu::aligned(8)]] insns; /* \(aqconst struct bpf_insn
*\(aq */
+uint64_t [[gnu::aligned(8)]] license; /* \(aqconst char *\(aq */
+uint32_t log_level; /* verbosity level of verifier
*/
+uint32_t log_size; /* size of user buffer */
+uint64_t [[gnu::aligned(8)]] log_buf; /* user supplied \(aqchar *\(aq
+ buffer */
+uint32_t kern_version;
+/* checked when
prog_type=kprobe
+ (since Linux 4.1) */
.\" commit 2541517c32be2531e0da59dfd7efc1ce844644f5
};
-} __attribute__((aligned(8)));
+} [[gnu::aligned(8)]];
.EE
.in
.\"
--
2.31.0
Ping: [PATCH v6] cacheflush.2: Document __builtin___clear_cache() as a more portable alternative
Ping On 12/15/20 2:30 PM, Alejandro Colomar wrote: > Reported-by: Heinrich Schuchardt > Signed-off-by: Alejandro Colomar > Cc: Martin Sebor > Cc: Dave Martin > --- > > v6: > - GCC has always exposed 'void *', as Martin Sebor noted. > It's Clang (and maybe others) that (following GCC's docs) > exposed 'char *'. > > man2/cacheflush.2 | 24 > 1 file changed, 24 insertions(+) > > diff --git a/man2/cacheflush.2 b/man2/cacheflush.2 > index aba625721..7a2eed506 100644 > --- a/man2/cacheflush.2 > +++ b/man2/cacheflush.2 > @@ -86,6 +86,30 @@ On Linux, this call first appeared on the MIPS > architecture, > but nowadays, Linux provides a > .BR cacheflush () > system call on some other architectures, but with different arguments. > +.SH NOTES > +Unless you need the finer grained control that this system call provides, > +you probably want to use the GCC built-in function > +.BR __builtin___clear_cache (), > +which provides a portable interface > +across platforms supported by GCC and compatible compilers: > +.PP > +.in +4n > +.EX > +.BI "void __builtin___clear_cache(void *" begin ", void *" end ); > +.EE > +.in > +.PP > +On platforms that don't require instruction cache flushes, > +.BR __builtin___clear_cache () > +has no effect. > +.PP > +.IR Note : > +On some GCC-compatible compilers, > +the prototype for this built-in function uses > +.I char * > +instead of > +.I void * > +for the parameters. > .SH BUGS > Linux kernels older than version 2.6.11 ignore the > .I addr > -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
[PATCH v6] cacheflush.2: Document __builtin___clear_cache() as a more portable alternative
Reported-by: Heinrich Schuchardt Signed-off-by: Alejandro Colomar Cc: Martin Sebor Cc: Dave Martin --- v6: - GCC has always exposed 'void *', as Martin Sebor noted. It's Clang (and maybe others) that (following GCC's docs) exposed 'char *'. man2/cacheflush.2 | 24 1 file changed, 24 insertions(+) diff --git a/man2/cacheflush.2 b/man2/cacheflush.2 index aba625721..7a2eed506 100644 --- a/man2/cacheflush.2 +++ b/man2/cacheflush.2 @@ -86,6 +86,30 @@ On Linux, this call first appeared on the MIPS architecture, but nowadays, Linux provides a .BR cacheflush () system call on some other architectures, but with different arguments. +.SH NOTES +Unless you need the finer grained control that this system call provides, +you probably want to use the GCC built-in function +.BR __builtin___clear_cache (), +which provides a portable interface +across platforms supported by GCC and compatible compilers: +.PP +.in +4n +.EX +.BI "void __builtin___clear_cache(void *" begin ", void *" end ); +.EE +.in +.PP +On platforms that don't require instruction cache flushes, +.BR __builtin___clear_cache () +has no effect. +.PP +.IR Note : +On some GCC-compatible compilers, +the prototype for this built-in function uses +.I char * +instead of +.I void * +for the parameters. .SH BUGS Linux kernels older than version 2.6.11 ignore the .I addr -- 2.29.2
[PATCH v5] cacheflush.2: Document __builtin___clear_cache() as a more portable alternative
Reported-by: Heinrich Schuchardt Signed-off-by: Alejandro Colomar --- man2/cacheflush.2 | 24 1 file changed, 24 insertions(+) diff --git a/man2/cacheflush.2 b/man2/cacheflush.2 index aba625721..7a2eed506 100644 --- a/man2/cacheflush.2 +++ b/man2/cacheflush.2 @@ -86,6 +86,30 @@ On Linux, this call first appeared on the MIPS architecture, but nowadays, Linux provides a .BR cacheflush () system call on some other architectures, but with different arguments. +.SH NOTES +Unless you need the finer grained control that this system call provides, +you probably want to use the GCC built-in function +.BR __builtin___clear_cache (), +which provides a portable interface +across platforms supported by GCC and compatible compilers: +.PP +.in +4n +.EX +.BI "void __builtin___clear_cache(void *" begin ", void *" end ); +.EE +.in +.PP +On platforms that don't require instruction cache flushes, +.BR __builtin___clear_cache () +has no effect. +.PP +.IR Note : +Until GCC 9.1.0, +the prototype for this built-in function used +.I char * +instead of +.I void * +for the parameters. .SH BUGS Linux kernels older than version 2.6.11 ignore the .I addr -- 2.29.2
Re: [PATCH v4 1/2] system_data_types.7: Add 'void *'
On 10/3/20 9:48 AM, G. Branden Robinson wrote: [...] >> The "short" answer[1] is that I think Alex is correct; Paul's caution is >> unwarranted and arises from confusion with the font alternation macros >> of the man(7) macro package. Examples of the latter are .BI and .BR. >> Those set their even-numbered arguments in one font and odd-numbered >> arguments in another, with no space between them. That suppression of >> space is the reason they exist. With the "single-font" macros like .B >> and .I[2], if you don't want space, don't type it. Hi Branden, This explanation is great :) Would you mind writing a patch with it? Cheers, Alex >> >> I could say more, including an annotated explanation of the groff and >> Version 7 Unix man(7) implementations of the I macro, if desired. :) :) >> >> Regards, >> Branden >> >> [1] since as everyone knows, I struggle with brevity >> [2] I (and others) discourage use of .SM and .SB because they can't be >> distinguished from ordinary roman and bold type, respectively, on >> terminals. >>
Re: [PATCH v4 1/2] system_data_types.7: Add 'void *'
Hi Michael and Branden, On 2020-10-03 09:48, G. Branden Robinson wrote: At 2020-10-03T09:10:14+0200, Michael Kerrisk (man-pages) wrote: On 10/2/20 10:27 PM, Alejandro Colomar wrote: On 2020-10-02 22:14, Paul Eggert wrote: > On 10/2/20 11:38 AM, Alejandro Colomar wrote: > >> .I void * >> >> renders with a space in between. > > That's odd, as "man(7)" says "All of the arguments will be > printed next to each other without intervening spaces". I'd play > it safe and quote the arg anyway. Oops, that's a bug in man(7). Don't worry about it. I'm not sure where that text in man(7) comes from. However, for clarity I would normally also use quotes in this case. Hi Michael and Branden, Here is the offending line: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/man.7#n172 Thanks, Alex Michael, you might want to have a look at it. I'll also add Branden, who might have something to say about it. Yes, maybe Branden can add some insight. The "short" answer[1] is that I think Alex is correct; Paul's caution is unwarranted and arises from confusion with the font alternation macros of the man(7) macro package. Examples of the latter are .BI and .BR. Those set their even-numbered arguments in one font and odd-numbered arguments in another, with no space between them. That suppression of space is the reason they exist. With the "single-font" macros like .B and .I[2], if you don't want space, don't type it. I could say more, including an annotated explanation of the groff and Version 7 Unix man(7) implementations of the I macro, if desired. :) Regards, Branden [1] since as everyone knows, I struggle with brevity [2] I (and others) discourage use of .SM and .SB because they can't be distinguished from ordinary roman and bold type, respectively, on terminals.
Re: [PATCH 1/4] system_data_types.7: Add '__int128'
Hi Paul, On 2020-10-02 22:19, Paul Eggert wrote: > On 10/2/20 1:03 PM, Alejandro Colomar wrote: >> I know it's not in stdint, >> but I mean that it behaves as any other stdint type. With caveats, of course. > > It doesn't. There's no portable way to use scanf and printf on it. I didn't need to. Yes that's a problem. It may be possible to write a custom specifier for printf, but I didn't try. I wrote one for printing binary, and it's not that difficult. If you really need it, this might help: https://github.com/alejandro-colomar/libalx/blob/d193b5648834c135824a5ba68d0ffcd2d38155a8/src/base/stdio/printf/b.c > You can't reliably convert it to intmax_t. Well, intmax_t isn't really that useful. I see it more like a generic type, than an actual type. I guess you could have typedef __int128 intwidest_t; if you find it's useful to you. > It doesn't have the associated _MIN and _MAX macros > that the stdint types do. It's a completely different animal. Those are really easy to write. For my use cases, they have been enough. These might be useful to you: #define UINT128_C(c)((uint128_t)c) #define INT128_C(c) (( int128_t)c) #define UINT128_MAX ((uint128_t)~UINT128_C(0)) #define INT128_MAX (( int128_t)(UINT128_MAX >> 1)) #define INT128_MIN (( int128_t)(-INT128_MAX - 1)) > > If all you need are a few bit-twiddling tricks on x86-64, it should > work. But watch out if you try to do something fancy, like multiply or > divide or read or print or atomics. There's a good reason it's excluded > from intmax_t. I know, they aren't perfect. But they are still very useful, and don't see a good reason to not document them. Cheers, Alex
Re: [PATCH v4 1/2] system_data_types.7: Add 'void *'
Hi Paul, On 2020-10-02 22:14, Paul Eggert wrote: > On 10/2/20 11:38 AM, Alejandro Colomar wrote: > >> .I void * >> >> renders with a space in between. > > That's odd, as "man(7)" says "All of the arguments will be printed next > to each other without intervening spaces". I'd play it safe and quote > the arg anyway. Oops, that's a bug in man(7). Don't worry about it. Michael, you might want to have a look at it. I'll also add Branden, who might have something to say about it. > >> > %p works with any object pointer type (or in POSIX, any pointer type), >> > not just void *. >> In theory, no (if otherwise, I'd like to know why): > > Oh, you're right. I had missed that. In GNU/Linux hosts, though, any > pointer (including function pointers) can be given to %p. > > The only platforms where %p wouldn't work on all pointers would be > platforms like IBM i, which has both 64-bit (process local) pointers and > 128-bit (tagged space) pointers and where you can declare and use > pointers of different widths in the same program. :-) Cheers, Alex
Re: [PATCH 1/4] system_data_types.7: Add '__int128'
Hi Paul, On 2020-10-02 21:54, Paul Eggert wrote: > On 10/2/20 12:01 PM, Alejandro Colomar wrote: >> If you propose not to document the stdint types either, > > This is not a stdint.h issue. __int128 is not in stdint.h and is not a > system data type in any real sense; it's purely a compiler issue. > Besides, do we start repeating the GCC manual too, while we're at it? At > some point we need to restrain ourselves and stay within the scope of > the man pages. I know it's not in stdint, but I mean that it behaves as any other stdint type. So I see value in having them documented together in the same page. And it's very useful in some (very specific) cases where portability isn't in mind (although many compilers are starting to provide this type). > > PS. Have you ever tried to use __int128 in real code? I have, to my > regret. It's a portability and bug minefield and should not be used > unless you really know what you're doing, which most people do not. I have. I used unsigned __int128, for operating on a big bit matrix. This type helped me remove a whole abstraction for the columns: unsigned __int128 mat[128]; // This is a 128x128 bit matrix. This way I avoided either having to use a shorter type, which would have been a bit weird: uint64_tmat[128][2];// This is more complicated to see. Or having to use GMP, which would have also complicated unnecessarily my code. And of course, I didn't care about portability, because I just wanted it to work. Thanks, Alex
[PATCH v5 1/2] system_data_types.7: Add 'void *'
Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add info about generic function parameters and return value Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add info about pointer artihmetic Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add Versions notes Compatibility between function pointers and void * hasn't always been so. Document when that was added to POSIX. Reported-by: Michael Kerrisk Signed-off-by: Alejandro Colomar system_data_types.7: void *: wfix Reported-by: Jonathan Wakely Reported-by: Paul Eggert Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 76 ++-- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index c82d3b388..7c1198802 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -679,7 +679,6 @@ See also the .I uintptr_t and .I void * -.\" TODO: Document void * types in this page. .RE .\"- lconv / @@ -1780,7 +1779,6 @@ See also the .I intptr_t and .I void * -.\" TODO: Document void * types in this page. .RE .\"- va_list --/ @@ -1814,6 +1812,80 @@ See also: .BR va_copy (3), .BR va_end (3) .RE +.\"- void * ---/ +.TP +.I void * +.RS +According to the C language standard, +a pointer to any object type may be converted to a pointer to +.I void +and back. +POSIX further requires that any pointer, +including pointers to functions, +may be converted to a pointer to +.I void +and back. +.PP +Conversions from and to any other pointer type are done implicitly, +not requiring casts at all. +Note that this feature prevents any kind of type checking: +the programmer should be careful not to convert a +.I void * +value to a type incompatible to that of the underlying data, +because that would result in undefined behavior. +.PP +This type is useful in function parameters and return value +to allow passing values of any type. +The function will typically use some mechanism to know +the real type of the data being passed via a pointer to +.IR void . +.PP +A value of this type can't be dereferenced, +as it would give a value of type +.IR void , +which is not possible. +Likewise, pointer arithmetic is not possible with this type. +However, in GNU C, pointer arithmetic is allowed +as an extension to the standard; +this is done by treating the size of a +.I void +or of a function as 1. +A consequence of this is that +.I sizeof +is also allowed on +.I void +and on function types, and returns 1. +.PP +The conversion specifier for +.I void * +for the +.BR printf (3) +and the +.BR scanf (3) +families of functions is +.BR p . +.PP +Versions: +The POSIX requirement about compatibility between +.I void * +and function pointers was added in +POSIX.1-2008 Technical Corrigendum 1 (2013). +.PP +Conforming to: +C99 and later; POSIX.1-2001 and later. +.PP +See also: +.BR malloc (3), +.BR memcmp (3), +.BR memcpy (3), +.BR memset (3) +.PP +See also the +.I intptr_t +and +.I uintptr_t +types in this page. +.RE .\"/ .SH NOTES The structures described in this manual page shall contain, -- 2.28.0
[PATCH v5 2/2] void.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/void.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/void.3 diff --git a/man3/void.3 b/man3/void.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/void.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
[PATCH v5 0/2] Document 'void *'
Hi Michael, Here I added a wfix fixing some wording issues and a few typos spotted by Paul and Jonathan in the (many) threads. As previously, it is squashed into a single commit. Thanks again for those who reviewed the patch! BTW, for those who don't have a local repo of the man-pages, below you can see a rendered version of the patch. Thanks, Alex [[ void * According to the C language standard, a pointer to any object type may be converted to a pointer to void and back. POSIX fur- ther requires that any pointer, including pointers to functions, may be converted to a pointer to void and back. Conversions from and to any other pointer type are done implic- itly, not requiring casts at all. Note that this feature pre- vents any kind of type checking: the programmer should be care- ful not to convert a void * value to a type incompatible to that of the underlying data, because that would result in undefined behavior. This type is useful in function parameters and return value to allow passing values of any type. The function will typically use some mechanism to know the real type of the data being passed via a pointer to void. A value of this type can't be dereferenced, as it would give a value of type void, which is not possible. Likewise, pointer arithmetic is not possible with this type. However, in GNU C, pointer arithmetic is allowed as an extension to the standard; this is done by treating the size of a void or of a function as 1. A consequence of this is that sizeof is also allowed on void and on function types, and returns 1. The conversion specifier for void * for the printf(3) and the scanf(3) families of functions is p. Versions: The POSIX requirement about compatibility between void * and function pointers was added in POSIX.1-2008 Technical Cor- rigendum 1 (2013). Conforming to: C99 and later; POSIX.1-2001 and later. See also: malloc(3), memcmp(3), memcpy(3), memset(3) See also the intptr_t and uintptr_t types in this page. ]] Alejandro Colomar (2): system_data_types.7: Add 'void *' void.3: New link to system_data_types(7) man3/void.3 | 1 + man7/system_data_types.7 | 76 ++-- 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 man3/void.3 -- 2.28.0
Re: [PATCH 1/4] system_data_types.7: Add '__int128'
Hi Paul, On 2020-10-02 19:52, Paul Eggert wrote: Why describe __int128_t in these man pages at all? __int128_t is not a property of either the kernel or of glibc, so it's out of scope. Well, as I see it, [unsigned] __int128 is as good as [u]int64_t. They are part of the C interface in Linux. As a programmer I never cared if it was Glibc providing me the type with a typedef, or GCC providing me the type with its magic. If you propose not to document the stdint types either, I can see your position, but I'll disagree. A few personal lines: I just want to use it, and to do that, I need a reference manual where to look for how to use it. And belive me that unsigned __int128 has been very useful to me. I think having this page with most of the types, in a centralized manner, is exactly what I would have needed in the past. I have had trouble finding where ssize_t was defined. I could have looked at the POSIX manual, and I would have easily found that it is defined in , but I didn't even know that it was a POSIX thing (and I can tell that I'm not the only one who didn't know this :), so I didn't even know where to search for it. When I wanted to use unsigned __128, kind of the same thing, where do you search for the documentation of something that you don't even know who specified it? When that happens, the first thing for me is to 'man something'. If that doesn't give me any useful info, then duckduckgo something. In the internet there's much info, but also much of it is incomplete or incorrect, so if I have the man, I trust the man over anything else (except for the standard documents, of course). But the standards documents usually provide the information in a reverse fashion: If you know where to look at, you'll find it. But if you only know its name, it'll be hard to find where it is. The man provides documentation with the name of what you want to know about. Simple and easy. And man is faster than the internet :) Regards, Alex.
Re: [PATCH v4 1/2] system_data_types.7: Add 'void *'
Hi Paul, On 2020-10-02 18:53, Paul Eggert wrote: > On 10/2/20 8:14 AM, Alejandro Colomar wrote: > >> +.I void * > > GNU style is a space between "void" and "*", so this should be '.I > "void\ *"', both here and elsewhere. The backslash prevents a line break. .I void * renders with a space in between. I'll show you the rendered version at the end of this email. > >> +Conversions from and to any other pointer type are done implicitly, >> +not requiring casts at all. >> +Note that this feature prevents any kind of type checking: >> +the programmer should be careful not to cast a > > Change "cast" to "convert", since the point is that no cast is needed. Ok. > >> +.PP >> +The conversion specifier for >> +.I void * >> +for the >> +.BR printf (3) >> +and the >> +.BR scanf (3) >> +families of functions is >> +.BR p ; >> +resulting commonly in >> +.B %p >> +for printing >> +.I void * >> +values. > > %p works with any object pointer type (or in POSIX, any pointer type), > not just void *. In theory, no (if otherwise, I'd like to know why): [[ p The argument shall be a pointer to void. The value of the pointer is converted to a sequence of printable characters, in an implementation-defined manner. ]] POSIX.1-2008 However, it's unlikely to cause any problems, I must admit. > > Should also mention "void const *", "void volatile *", etc. I already answered to this: https://lore.kernel.org/linux-man/cah6ehdqhh46tjvc72mewftwci7iouaod0ic1zlrga+c-36g...@mail.gmail.com/T/#m6f657e988558a556cb70f7c056ef7a24e73dbe4a > Plus it > really should talk about plain "void", saying that it's a placeholder as > a return value for functions, for casting away values, and as a keyword > in C11 for functions with no parameters (though this is being changed in > the next C version!). I sent comments about most of this stuff already. 'void' is a completely different type from 'void *'. This patch is for 'void *'. If 'void' is documented, it'll be in a different entry (although in the same page), and therefore, that'll be for a different patch. Thanks, Alex __ void * According to the C language standard, a pointer to any object type may be converted to a pointer to void and back. POSIX fur- ther requires that any pointer, including pointers to functions, may be converted to a pointer to void and back. Conversions from and to any other pointer type are done implic- itly, not requiring casts at all. Note that this feature pre- vents any kind of type checking: the programmer should be care- ful not to cast a void * value to a type incompatible to that of the underlying data, because that would result in undefined be- havior. This type is useful in function parameters and return value to allow passing values of any type. The function will usually use some mechanism to know of which type the underlying data passed to the function really is. A value of this type can't be dereferenced, as it would give a value of type void which is not possible. Likewise, pointer arithmetic is not possible with this type. However, in GNU C, poitner arithmetic is allowed as an extension to the standard; this is done by treating the size of a void or of a function as 1. A consequence of this is that sizeof is also allowed on void and on function types, and returns 1. The conversion specifier for void * for the printf(3) and the scanf(3) families of functions is p; resulting commonly in %p for printing void * values. Versions: The POSIX requirement about compatibility between void * and function pointers was added in POSIX.1-2008 Technical Cor- rigendum 1 (2013). Conforming to: C99 and later; POSIX.1-2001 and later. See also: malloc(3), memcmp(3), memcpy(3), memset(3) See also the intptr_t and uintptr_t types in this page.
[PATCH v4 2/2] void.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/void.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/void.3 diff --git a/man3/void.3 b/man3/void.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/void.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
[PATCH v4 1/2] system_data_types.7: Add 'void *'
Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add info about generic function parameters and return value Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add info about pointer artihmetic Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add Versions notes Compatibility between function pointers and void * hasn't always been so. Document when that was added to POSIX. Reported-by: Michael Kerrisk Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 80 +++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index c82d3b388..277e05b12 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -679,7 +679,6 @@ See also the .I uintptr_t and .I void * -.\" TODO: Document void * types in this page. .RE .\"- lconv / @@ -1780,7 +1779,6 @@ See also the .I intptr_t and .I void * -.\" TODO: Document void * types in this page. .RE .\"- va_list --/ @@ -1814,6 +1812,84 @@ See also: .BR va_copy (3), .BR va_end (3) .RE +.\"- void * ---/ +.TP +.I void * +.RS +According to the C language standard, +a pointer to any object type may be converted to a pointer to +.I void +and back. +POSIX further requires that any pointer, +including pointers to functions, +may be converted to a pointer to +.I void +and back. +.PP +Conversions from and to any other pointer type are done implicitly, +not requiring casts at all. +Note that this feature prevents any kind of type checking: +the programmer should be careful not to cast a +.I void * +value to a type incompatible to that of the underlying data, +because that would result in undefined behavior. +.PP +This type is useful in function parameters and return value +to allow passing values of any type. +The function will usually use some mechanism to know +of which type the underlying data passed to the function really is. +.PP +A value of this type can't be dereferenced, +as it would give a value of type +.I void +which is not possible. +Likewise, pointer arithmetic is not possible with this type. +However, in GNU C, poitner arithmetic is allowed +as an extension to the standard; +this is done by treating the size of a +.I void +or of a function as 1. +A consequence of this is that +.I sizeof +is also allowed on +.I void +and on function types, and returns 1. +.PP +The conversion specifier for +.I void * +for the +.BR printf (3) +and the +.BR scanf (3) +families of functions is +.BR p ; +resulting commonly in +.B %p +for printing +.I void * +values. +.PP +Versions: +The POSIX requirement about compatibility between +.I void * +and function pointers was added in +POSIX.1-2008 Technical Corrigendum 1 (2013). +.PP +Conforming to: +C99 and later; POSIX.1-2001 and later. +.PP +See also: +.BR malloc (3), +.BR memcmp (3), +.BR memcpy (3), +.BR memset (3) +.PP +See also the +.I intptr_t +and +.I uintptr_t +types in this page. +.RE .\"/ .SH NOTES The structures described in this manual page shall contain, -- 2.28.0
[PATCH v4 0/2] Document 'void *'
Hi Michael, I'm sorry I forgot to increase the version count. And given there are conversations continuing in old threads, you may mix them easily. I'm a bit lost in the emails too. I'll resend the latest patch (identically) as v4 (there's no v3, but this is the 4th time or so, so let's call it v4). Regards, Alex Alejandro Colomar (2): system_data_types.7: Add 'void *' void.3: New link to system_data_types(7) man3/void.3 | 1 + man7/system_data_types.7 | 80 +++- 2 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 man3/void.3 -- 2.28.0
[PATCH v2 2/4] __int128.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/__int128.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/__int128.3 diff --git a/man3/__int128.3 b/man3/__int128.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/__int128.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
[PATCH v2 4/4] unsigned-__int128.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/unsigned-__int128.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/unsigned-__int128.3 diff --git a/man3/unsigned-__int128.3 b/man3/unsigned-__int128.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/unsigned-__int128.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
[PATCH v2 3/4] system_data_types.7: Add 'unsigned __int128'
Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 35 +++ 1 file changed, 35 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index 5f9aa648f..3cf3f0ec9 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -1822,6 +1822,41 @@ and .I void * types in this page. .RE +.\"- unsigned __int128 / +.TP +.I unsigned __int128 +.RS +An unsigned integer type +of a fixed width of exactly 128 bits. +.PP +When using GCC, +it is supported only for targets where +the compiler is able to generate efficient code for 128-bit arithmetic. +.PP +Versions: +GCC 4.6.0 and later. +.PP +Conforming to: +This is a non-standard extension, present in GCC. +It is not standardized by the C language standard nor POSIX. +.PP +Notes: +This type is available without including any header. +.PP +Bugs: +It is not possible to express an integer constant of type +.I unsigned __int128 +in implementations where +.I unsigned long long +is less than 128 bits wide. +.PP +See also the +.IR __int128 , +.I uintmax_t +and +.IR uint N _t +types in this page. +.RE .\"- va_list --/ .TP .I va_list -- 2.28.0
[PATCH v2 1/4] system_data_types.7: Add '__int128'
Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 40 1 file changed, 40 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index e545aa1a0..5f9aa648f 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -40,6 +40,8 @@ system_data_types \- overview of system data types .\"* Description (no "Description" header) .\"A few lines describing the type. .\" +.\"* Versions (optional) +.\" .\"* Conforming to (see NOTES) .\"Format: CXY and later; POSIX.1- and later. .\" @@ -48,6 +50,44 @@ system_data_types \- overview of system data types .\"* Bugs (if any) .\" .\"* See also +.\"- __int128 -/ +.TP +.I __int128 +.RS +.RI [ signed ] +.I __int128 +.PP +A signed integer type +of a fixed width of exactly 128 bits. +.PP +When using GCC, +it is supported only for targets where +the compiler is able to generate efficient code for 128-bit arithmetic. +.PP +Versions: +GCC 4.6.0 and later. +.PP +Conforming to: +This is a non-standard extension, present in GCC. +It is not standardized by the C language standard nor POSIX. +.PP +Notes: +This type is available without including any header. +.PP +Bugs: +It is not possible to express an integer constant of type +.I __int128 +in implementations where +.I long long +is less than 128 bits wide. +.PP +See also the +.IR intmax_t , +.IR int N _t +and +.I unsigned __int128 +types in this page. +.RE .\"- aiocb / .TP .I aiocb -- 2.28.0
[PATCH v2 0/4] Document 128-bit types
Hi Michael, I fixed the stray '"' noticed by Florian. Cheers, Alex Alejandro Colomar (4): system_data_types.7: Add '__int128' __int128.3: New link to system_data_types(7) system_data_types.7: Add 'unsigned __int128' unsigned-__int128.3: New link to system_data_types(7) man3/__int128.3 | 1 + man3/unsigned-__int128.3 | 1 + man7/system_data_types.7 | 75 3 files changed, 77 insertions(+) create mode 100644 man3/__int128.3 create mode 100644 man3/unsigned-__int128.3 -- 2.28.0
[PATCH 2/2] void.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/void.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/void.3 diff --git a/man3/void.3 b/man3/void.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/void.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
[PATCH 1/2] system_data_types.7: Add 'void *'
Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add info about generic function parameters and return value Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add info about pointer artihmetic Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar system_data_types.7: void *: Add Versions notes Compatibility between function pointers and void * hasn't always been so. Document when that was added to POSIX. Reported-by: Michael Kerrisk Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 80 +++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index c82d3b388..277e05b12 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -679,7 +679,6 @@ See also the .I uintptr_t and .I void * -.\" TODO: Document void * types in this page. .RE .\"- lconv / @@ -1780,7 +1779,6 @@ See also the .I intptr_t and .I void * -.\" TODO: Document void * types in this page. .RE .\"- va_list --/ @@ -1814,6 +1812,84 @@ See also: .BR va_copy (3), .BR va_end (3) .RE +.\"- void * ---/ +.TP +.I void * +.RS +According to the C language standard, +a pointer to any object type may be converted to a pointer to +.I void +and back. +POSIX further requires that any pointer, +including pointers to functions, +may be converted to a pointer to +.I void +and back. +.PP +Conversions from and to any other pointer type are done implicitly, +not requiring casts at all. +Note that this feature prevents any kind of type checking: +the programmer should be careful not to cast a +.I void * +value to a type incompatible to that of the underlying data, +because that would result in undefined behavior. +.PP +This type is useful in function parameters and return value +to allow passing values of any type. +The function will usually use some mechanism to know +of which type the underlying data passed to the function really is. +.PP +A value of this type can't be dereferenced, +as it would give a value of type +.I void +which is not possible. +Likewise, pointer arithmetic is not possible with this type. +However, in GNU C, poitner arithmetic is allowed +as an extension to the standard; +this is done by treating the size of a +.I void +or of a function as 1. +A consequence of this is that +.I sizeof +is also allowed on +.I void +and on function types, and returns 1. +.PP +The conversion specifier for +.I void * +for the +.BR printf (3) +and the +.BR scanf (3) +families of functions is +.BR p ; +resulting commonly in +.B %p +for printing +.I void * +values. +.PP +Versions: +The POSIX requirement about compatibility between +.I void * +and function pointers was added in +POSIX.1-2008 Technical Corrigendum 1 (2013). +.PP +Conforming to: +C99 and later; POSIX.1-2001 and later. +.PP +See also: +.BR malloc (3), +.BR memcmp (3), +.BR memcpy (3), +.BR memset (3) +.PP +See also the +.I intptr_t +and +.I uintptr_t +types in this page. +.RE .\"/ .SH NOTES The structures described in this manual page shall contain, -- 2.28.0
[PATCH 0/2] Document 'void *'
Hi Michael, As you asked, I squashed. And added the POSIX.1-2008 note too. Thanks for that! Cheers, Alex Alejandro Colomar (2): system_data_types.7: Add 'void *' void.3: New link to system_data_types(7) man3/void.3 | 1 + man7/system_data_types.7 | 80 +++- 2 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 man3/void.3 -- 2.28.0
Re: [PATCH 2/2] system_data_types.7: void *: Add info about pointer artihmetic
Hi Michael, The 2/2 is a typo. This is a standalone patch. Cheers, Alex On 2020-10-02 11:44, Alejandro Colomar wrote: Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar --- Paul and David, Thanks for your input! Alex man7/system_data_types.7 | 11 +++ 1 file changed, 11 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index 6451782db..bc7d5a8a0 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -1918,6 +1918,17 @@ A value of this type can't be dereferenced, as it would give a value of type .I void which is not possible. +Likewise, pointer arithmetic is not possible with this type. +However, in GNU C, poitner arithmetic is allowed +as an extension to the standard; +this is done by treating the size of a +.I void +or of a function as 1. +A consequence of this is that +.I sizeof +is also allowed on +.I void +and on function types, and returns 1. .PP The conversion specifier for .I void *
[PATCH 2/2] system_data_types.7: void *: Add info about pointer artihmetic
Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar --- Paul and David, Thanks for your input! Alex man7/system_data_types.7 | 11 +++ 1 file changed, 11 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index 6451782db..bc7d5a8a0 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -1918,6 +1918,17 @@ A value of this type can't be dereferenced, as it would give a value of type .I void which is not possible. +Likewise, pointer arithmetic is not possible with this type. +However, in GNU C, poitner arithmetic is allowed +as an extension to the standard; +this is done by treating the size of a +.I void +or of a function as 1. +A consequence of this is that +.I sizeof +is also allowed on +.I void +and on function types, and returns 1. .PP The conversion specifier for .I void * -- 2.28.0
[PATCH] system_data_types.7: void *: Add info about generic function parameters and return value
Reported-by: Paul Eggert Reported-by: David Laight Signed-off-by: Alejandro Colomar --- Paul and David, Thanks for your input! Alex man7/system_data_types.7 | 16 1 file changed, 16 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index aab64e002..6451782db 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -1903,6 +1903,16 @@ and back. .PP Conversions from and to any other pointer type are done implicitly, not requiring casts at all. +Note that this feature prevents any kind of type checking: +the programmer should be careful not to cast a +.I void * +value to a type incompatible to that of the underlying data, +because that would result in undefined behavior. +.PP +This type is useful in function parameters and return value +to allow passing values of any type. +The function will usually use some mechanism to know +of which type the underlying data passed to the function really is. .PP A value of this type can't be dereferenced, as it would give a value of type @@ -1926,6 +1936,12 @@ values. Conforming to: C99 and later; POSIX.1-2001 and later. .PP +See also: +.BR malloc (3), +.BR memcmp (3), +.BR memcpy (3), +.BR memset (3) +.PP See also the .I intptr_t and -- 2.28.0
[PATCH 1/4] system_data_types.7: Add '__int128'
Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 40 1 file changed, 40 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index e545aa1a0..5f9aa648f 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -40,6 +40,8 @@ system_data_types \- overview of system data types .\"* Description (no "Description" header) .\"A few lines describing the type. .\" +.\"* Versions (optional) +.\" .\"* Conforming to (see NOTES) .\"Format: CXY and later; POSIX.1- and later. .\" @@ -48,6 +50,44 @@ system_data_types \- overview of system data types .\"* Bugs (if any) .\" .\"* See also +.\"- __int128 -/ +.TP +.I __int128 +.RS +.RI [ signed ] +.I __int128 +.PP +A signed integer type +of a fixed width of exactly 128 bits. +.PP +When using GCC, +it is supported only for targets where +the compiler is able to generate efficient code for 128-bit arithmetic". +.PP +Versions: +GCC 4.6.0 and later. +.PP +Conforming to: +This is a non-standard extension, present in GCC. +It is not standardized by the C language standard nor POSIX. +.PP +Notes: +This type is available without including any header. +.PP +Bugs: +It is not possible to express an integer constant of type +.I __int128 +in implementations where +.I long long +is less than 128 bits wide. +.PP +See also the +.IR intmax_t , +.IR int N _t +and +.I unsigned __int128 +types in this page. +.RE .\"- aiocb / .TP .I aiocb -- 2.28.0
[PATCH 2/4] __int128.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/__int128.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/__int128.3 diff --git a/man3/__int128.3 b/man3/__int128.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/__int128.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
[PATCH 3/4] system_data_types.7: Add 'unsigned __int128'
Signed-off-by: Alejandro Colomar --- man7/system_data_types.7 | 35 +++ 1 file changed, 35 insertions(+) diff --git a/man7/system_data_types.7 b/man7/system_data_types.7 index 5f9aa648f..3cf3f0ec9 100644 --- a/man7/system_data_types.7 +++ b/man7/system_data_types.7 @@ -1822,6 +1822,41 @@ and .I void * types in this page. .RE +.\"- unsigned __int128 / +.TP +.I unsigned __int128 +.RS +An unsigned integer type +of a fixed width of exactly 128 bits. +.PP +When using GCC, +it is supported only for targets where +the compiler is able to generate efficient code for 128-bit arithmetic". +.PP +Versions: +GCC 4.6.0 and later. +.PP +Conforming to: +This is a non-standard extension, present in GCC. +It is not standardized by the C language standard nor POSIX. +.PP +Notes: +This type is available without including any header. +.PP +Bugs: +It is not possible to express an integer constant of type +.I unsigned __int128 +in implementations where +.I unsigned long long +is less than 128 bits wide. +.PP +See also the +.IR __int128 , +.I uintmax_t +and +.IR uint N _t +types in this page. +.RE .\"- va_list --/ .TP .I va_list -- 2.28.0
[PATCH 0/4] Document 128-bit types
Hi Michael, I think this might be ready for a patch. I'm done for today :-) Cheers, Alex Alejandro Colomar (4): system_data_types.7: Add '__int128' __int128.3: New link to system_data_types(7) system_data_types.7: Add 'unsigned __int128' unsigned-__int128.3: New link to system_data_types(7) man3/__int128.3 | 1 + man3/unsigned-__int128.3 | 1 + man7/system_data_types.7 | 75 3 files changed, 77 insertions(+) create mode 100644 man3/__int128.3 create mode 100644 man3/unsigned-__int128.3 -- 2.28.0
[PATCH 4/4] unsigned-__int128.3: New link to system_data_types(7)
Signed-off-by: Alejandro Colomar --- man3/unsigned-__int128.3 | 1 + 1 file changed, 1 insertion(+) create mode 100644 man3/unsigned-__int128.3 diff --git a/man3/unsigned-__int128.3 b/man3/unsigned-__int128.3 new file mode 100644 index 0..db50c0f09 --- /dev/null +++ b/man3/unsigned-__int128.3 @@ -0,0 +1 @@ +.so man7/system_data_types.7 -- 2.28.0
