[Mesa-dev] [PATCH 24/24] gallium/aux/os: Fix warning/handle failure to read from /proc/self/cmdline

[Emil Velikov]

On 5 June 2018 at 12:59, Gert Wollny <gert.wollny at collabora.com> wrote:
> Handle the failure to read from /proc/self/cmdline by printing an error
> message and fix the -Wsign-compare warning:
>
> In file included from ./util/u_memory.h:39:0,
>                  from os/os_process.c:31:
> os/os_process.c: In function 'os_get_command_line':
> os/os_process.c:140:16: warning: comparison between signed and unsigned
> integer expressions [-Wsign-compare]
>        assert(n < size);
>                 ^
> ./util/u_debug.h:189:30: note: in definition of macro 'debug_assert'
>  #define debug_assert(expr) ((expr) ? (void)0 : _debug_assert_fail(#expr,
> __FILE__, __LINE__, __FUNCTION__))
>                               ^~~~
> os/os_process.c:140:7: note: in expansion of macro 'assert'
>        assert(n < size);
>        ^~~~~~
>
> gallium/aux/os/ squash
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/gallium/auxiliary/os/os_process.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/auxiliary/os/os_process.c b/src/gallium/auxiliary/os/os_process.c
> index 035bd228e7..a95465a7bb 100644
> --- a/src/gallium/auxiliary/os/os_process.c
> +++ b/src/gallium/auxiliary/os/os_process.c
> @@ -34,6 +34,7 @@
>  #  include <windows.h>
>  #elif defined(__GLIBC__) || defined(__CYGWIN__)
>  #  include <errno.h>
> +#  include <stdio.h>
>  #elif defined(PIPE_OS_BSD) || defined(PIPE_OS_APPLE) || defined(PIPE_OS_ANDROID)
>  #  include <stdlib.h>
>  #elif defined(PIPE_OS_HAIKU)
> @@ -135,19 +136,26 @@ os_get_command_line(char *cmdline, size_t size)
>  #elif defined(PIPE_OS_LINUX)
>     int f = open("/proc/self/cmdline", O_RDONLY);
>     if (f) {
> -      const int n = read(f, cmdline, size - 1);
> -      int i;
> -      assert(n < size);
> -      // The arguments are separated by '\0' chars.  Convert them to spaces.
> -      for (i = 0; i < n; i++) {
> -         if (cmdline[i] == 0) {
> -            cmdline[i] = ' ';
> +      bool retval;
> +      const ssize_t n = read(f, cmdline, size - 1);
> +      if (n >= 0) {
> +        int i;
> +        assert(n < (ssize_t)size);
> +        // The arguments are separated by '\0' chars.  Convert them to spaces.
> +        for (i = 0; i < n; i++) {
> +           if (cmdline[i] == 0) {
> +              cmdline[i] = ' ';
> +           }
>           }
> +        // terminate the string
> +        cmdline[n] = 0;
> +        retval = TRUE;
> +      } else {
> +        perror("os_get_command_line: Error reading from /proc/self/cmdline:");
> +        retval = FALSE;
>        }
> -      // terminate the string
> -      cmdline[n] = 0;
>        close(f);
> -      return TRUE;
> +      return retval;

As I tend to be a sucker for smaller code and short patches, here is
one alternative.
Note: the perror is explicitly omitted, since libraries shouldn't
print too much to stderr/stdout.

diff --git a/src/gallium/auxiliary/os/os_process.c
b/src/gallium/auxiliary/os/os_process.c
index 035bd228e76..1765272377e 100644
--- a/src/gallium/auxiliary/os/os_process.c
+++ b/src/gallium/auxiliary/os/os_process.c
@@ -135,7 +135,11 @@ os_get_command_line(char *cmdline, size_t size)
 #elif defined(PIPE_OS_LINUX)
    int f = open("/proc/self/cmdline", O_RDONLY);
    if (f) {
-      const int n = read(f, cmdline, size - 1);
+      const ssize_t n = read(f, cmdline, size - 1);
+      if (n == -1) {
+         close(f);
+         return FALSE;
+      }
       int i;
       assert(n < size);

-Emil