Re: [PATCH] [PR symtab/17911] Recognize bad file types

[Pedro Alves <palves at redhat dot com>]

On 09/19/2015 11:40 PM, Doug Evans wrote:

> diff --git a/gdb/source.c b/gdb/source.c
> index fab974c..34384fa 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -684,7 +684,10 @@ source_info (char *ignore, int from_tty)
>  }
>  
>  
> -/* Return True if the file NAME exists and is a regular file.  */
> +/* Return True if the file NAME exists and is a regular file.
> +   If the result is false then errno is set to a useful value assuming we're
> +   expecting a regular file.  */
> +
>  static int
>  is_regular_file (const char *name)
>  {
> @@ -699,7 +702,14 @@ is_regular_file (const char *name)
>    if (status != 0)
>      return (errno != ENOENT);
>  
> -  return S_ISREG (st.st_mode);
> +  if (S_ISREG (st.st_mode))
> +    return 1;
> +
> +  if (S_ISDIR (st.st_mode))
> +    errno = EISDIR;
> +  else
> +    errno = EINVAL;
> +  return 0;
>  }
>  
>  /* Open a file named STRING, searching path PATH (dir names sep by some char)
> @@ -785,6 +795,7 @@ openp (const char *path, int opts, const char *string,
>  	{
>  	  filename = NULL;
>  	  fd = -1;
> +	  /* Note: is_regular_file will have set errno appropriately.  */
>  	}
>  
>        if (!(opts & OPF_SEARCH_IN_PATH))
> @@ -808,6 +819,7 @@ openp (const char *path, int opts, const char *string,
>    alloclen = strlen (path) + strlen (string) + 2;
>    filename = alloca (alloclen);
>    fd = -1;
> +  errno = ENOENT;
>  
>    dir_vec = dirnames_to_char_ptr_vec (path);
>    back_to = make_cleanup_free_char_ptr_vec (dir_vec);
> @@ -879,6 +891,10 @@ openp (const char *path, int opts, const char *string,
>  	  if (fd >= 0)
>  	    break;
>  	}
> +      else
> +	{
> +	  /* Note: is_regular_file will have set errno appropriately.  */
> +	}
>      }

Seems like there are function calls after these that may
clobber errno.  I think it'd be safer to do the usual
save_errno = errno; / errno = save_errno; dance.

> +# Test passing bad files to gdb.  PR symtab/17911
> +
> +# We watch for specific text for errno, so only test on systems we have
> +# confidence in the error text.
> +
> +if { ! [ishost "*-*-linux*"] } {
> +  return 0
> +}

Same comment as to the other patch -- I think we should instead
remove this check and if the error message is different on other
hosts, then whoever cares about such hosts will adjust the
test.  I don't think there will be that many cases of
different messages.

Otherwise LGTM.

Thanks,
Pedro Alves