Re: [PATCH 2/2] Use enum types for remote fileio flags

[Simon Marchi <simark@simark.ca>]

On 2/25/26 4:25 PM, Tom Tromey wrote:
> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index 6919a1c37d3..6733aa7cc48 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -287,8 +287,9 @@ adi_tag_fd ()
>    char cl_name[MAX_PROC_NAME_SIZE];
>    snprintf (cl_name, sizeof(cl_name), "/proc/%ld/adi/tags", (long) pid);
>    fileio_error target_errno;
> -  proc->stat.tag_fd = target_fileio_open (NULL, cl_name, O_RDWR|O_EXCL,
> -					  false, 0, &target_errno);
> +  proc->stat.tag_fd = target_fileio_open (NULL, cl_name,
> +					  FILEIO_O_RDWR | FILEIO_O_EXCL,
> +					  FILEIO_S_IRWXU, 0, &target_errno);

Perhaps change that 0 to false (the `bool warn_if_slow` param).

So the flags here goes from "false" (obviously wrong) to FILEIO_S_IRWXU.
Since we don't pass the _CREATE flag, I think that the value for mode
is irrelevant.  If so, it might be clearer to pass 0, otherwise it makes
it seem like the value we pass is meaningful, which can be confusing.

>  /* lseek(2) flags */
> -#define FILEIO_SEEK_SET             0
> -#define FILEIO_SEEK_CUR             1
> -#define FILEIO_SEEK_END             2
> +enum fileio_lseek_flag : unsigned
> +{
> +  FILEIO_SEEK_SET = 0,
> +  FILEIO_SEEK_CUR = 1,
> +  FILEIO_SEEK_END = 2,
> +};
> +DEF_ENUM_FLAGS_TYPE (enum fileio_lseek_flag, fileio_lseek_flags);

I don't think these values are actually flags, theys are all mutually
exclusive, I think it should be a regular enum (or enum class).

If we want to mimic the posix API, you could call it
"fileio_lseek_whence".

But I also noticed the enum type is not actually used.  It could be used
in remote_fileio_seek_flag_to_host, although it wouldn't be terribly
useful.  So perhaps this enum could be anonymous actually.

>  
>  /* errno values */
>  enum fileio_error
> @@ -146,12 +159,12 @@ extern int fileio_error_to_host (fileio_error errnum);
>  /* Convert File-I/O open flags FFLAGS to host format, storing
>     the result in *FLAGS.  Return 0 on success, -1 on error.  */
>  
> -extern int fileio_to_host_openflags (int fflags, int *flags);
> +extern int fileio_to_host_openflags (fileio_open_flags fflags, int *flags);

In the implementation, the second parameter is named open_flags_p.  You
could maybe rename it here.  That would free up "flags" for the first
parameter, if you want.

Simon