diff mbox

libc: The arguments of puts() can be marked as "const"

Message ID 1496828499-30874-1-git-send-email-thuth@redhat.com
State Accepted
Headers show

Commit Message

Thomas Huth June 7, 2017, 9:41 a.m. UTC
puts() does not change the string, so the parameter can be "const".

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 lib/libc/include/stdio.h | 2 +-
 lib/libc/stdio/puts.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Alexey Kardashevskiy June 8, 2017, 6:12 a.m. UTC | #1
On 07/06/17 19:41, Thomas Huth wrote:
> puts() does not change the string, so the parameter can be "const".
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Thanks, applied.


While you are fixing stuff like this, I started getting:

/home/aik/p/slof/slof/paflof.c: In function ‘engine’:
/home/aik/p/slof/slof/paflof.c:84:23: warning: array subscript is below
array bounds [-Warray-bounds]
   dp = the_data_stack - 1;
        ~~~~~~~~~~~~~~~^~~
/home/aik/p/slof/slof/paflof.c:85:22: warning: array subscript is below
array bounds [-Warray-bounds]
   rp = handler_stack - 1;
        ~~~~~~~~~~~~~~^~~


with gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1) from Fedora24/BE.

Can you please take a look on this? Thanks.


> ---
>  lib/libc/include/stdio.h | 2 +-
>  lib/libc/stdio/puts.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libc/include/stdio.h b/lib/libc/include/stdio.h
> index 84cddea..c54528f 100644
> --- a/lib/libc/include/stdio.h
> +++ b/lib/libc/include/stdio.h
> @@ -51,7 +51,7 @@ int setvbuf(FILE *stream, char *buf, int mode , size_t size);
>  
>  int putc(int ch, FILE *stream);
>  int putchar(int ch);
> -int puts(char *str);
> +int puts(const char *str);
>  
>  int scanf(const char *format, ...)  __attribute__((format (scanf, 1, 2)));
>  int fscanf(FILE *stream, const char *format, ...) __attribute__((format (scanf, 2, 3)));
> diff --git a/lib/libc/stdio/puts.c b/lib/libc/stdio/puts.c
> index 3f48dbf..9a93008 100644
> --- a/lib/libc/stdio/puts.c
> +++ b/lib/libc/stdio/puts.c
> @@ -17,7 +17,7 @@
>  
>  
>  int
> -puts(char *str)
> +puts(const char *str)
>  {
>  	int ret;
>  
>
Thomas Huth June 8, 2017, 6:54 a.m. UTC | #2
On 08.06.2017 08:12, Alexey Kardashevskiy wrote:
[...]
> While you are fixing stuff like this, I started getting:
> 
> /home/aik/p/slof/slof/paflof.c: In function ‘engine’:
> /home/aik/p/slof/slof/paflof.c:84:23: warning: array subscript is below
> array bounds [-Warray-bounds]
>    dp = the_data_stack - 1;
>         ~~~~~~~~~~~~~~~^~~
> /home/aik/p/slof/slof/paflof.c:85:22: warning: array subscript is below
> array bounds [-Warray-bounds]
>    rp = handler_stack - 1;
>         ~~~~~~~~~~~~~~^~~
> 
> with gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1) from Fedora24/BE.
> 
> Can you please take a look on this? Thanks.

See Segher's suggestions here:

 https://lists.ozlabs.org/pipermail/slof/2016-August/001221.html

IMHO we could also just simply sacrifice the first stack entry and only
use "dp = the_data_stack", without adding the inline asm here (to keep
paflof.c portable).

 Thomas
Segher Boessenkool June 8, 2017, 5:13 p.m. UTC | #3
On Thu, Jun 08, 2017 at 08:54:06AM +0200, Thomas Huth wrote:
> On 08.06.2017 08:12, Alexey Kardashevskiy wrote:
> > /home/aik/p/slof/slof/paflof.c: In function ‘engine’:
> > /home/aik/p/slof/slof/paflof.c:84:23: warning: array subscript is below
> > array bounds [-Warray-bounds]
> >    dp = the_data_stack - 1;
> >         ~~~~~~~~~~~~~~~^~~
> > /home/aik/p/slof/slof/paflof.c:85:22: warning: array subscript is below
> > array bounds [-Warray-bounds]
> >    rp = handler_stack - 1;
> >         ~~~~~~~~~~~~~~^~~
> > 
> > with gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1) from Fedora24/BE.
> > 
> > Can you please take a look on this? Thanks.
> 
> See Segher's suggestions here:
> 
>  https://lists.ozlabs.org/pipermail/slof/2016-August/001221.html
> 
> IMHO we could also just simply sacrifice the first stack entry and only
> use "dp = the_data_stack", without adding the inline asm here (to keep
> paflof.c portable).

Or you simply make the_*_stack declared as a pointer in paflof.c, not as
an array.  Where is it actually defined?  If it is defined in assembler
code all is fine; if it is defined in C code, well, don't lie to the
compiler or it will take its revenge (if using full-program optimisation
it can still see you're accessing the array out-of-bounds for example,
or worse, assume you don't do undefined things and optimise accordingly).
An easy way around is to have the_*_stack just an external symbol, and
have the_real_*_stack for the arrays of cells, and then equate the two
in a linker script.

Or, ignore the warning.  If ever things break (and they won't), it will
do so with lots of fireworks; it won't silently break.


Segher
diff mbox

Patch

diff --git a/lib/libc/include/stdio.h b/lib/libc/include/stdio.h
index 84cddea..c54528f 100644
--- a/lib/libc/include/stdio.h
+++ b/lib/libc/include/stdio.h
@@ -51,7 +51,7 @@  int setvbuf(FILE *stream, char *buf, int mode , size_t size);
 
 int putc(int ch, FILE *stream);
 int putchar(int ch);
-int puts(char *str);
+int puts(const char *str);
 
 int scanf(const char *format, ...)  __attribute__((format (scanf, 1, 2)));
 int fscanf(FILE *stream, const char *format, ...) __attribute__((format (scanf, 2, 3)));
diff --git a/lib/libc/stdio/puts.c b/lib/libc/stdio/puts.c
index 3f48dbf..9a93008 100644
--- a/lib/libc/stdio/puts.c
+++ b/lib/libc/stdio/puts.c
@@ -17,7 +17,7 @@ 
 
 
 int
-puts(char *str)
+puts(const char *str)
 {
 	int ret;