Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: Simon Marchi <simark@simark.ca>
Cc: Tom Tromey <tromey@adacore.com>,  gdb-patches@sourceware.org
Subject: Re: [PATCH 2/2] Use enum types for remote fileio flags
Date: Fri, 27 Feb 2026 07:15:17 -0700	[thread overview]
Message-ID: <87jyvyuvbe.fsf@tromey.com> (raw)
In-Reply-To: <d633c702-f6e8-4723-b634-247bd480298e@simark.ca> (Simon Marchi's message of "Thu, 26 Feb 2026 12:10:58 -0500")

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +  proc->stat.tag_fd = target_fileio_open (NULL, cl_name,
>> +					  FILEIO_O_RDWR | FILEIO_O_EXCL,
>> +					  FILEIO_S_IRWXU, 0, &target_errno);

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

I added a patch to fix up the warn_if_slow stuff.

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

I fixed this.

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

Yeah I did this, thanks.

>> 
>> /* 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);

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

I made them use the same names.  I didn't change it to 'flags' though.

Tom

      reply	other threads:[~2026-02-27 14:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25 21:25 [PATCH 0/2] Stronger typing for remote file i/o Tom Tromey
2026-02-25 21:25 ` [PATCH 1/2] Use a newtype for remote file descriptor Tom Tromey
2026-02-26 16:38   ` Simon Marchi
2026-02-27 13:36     ` Tom Tromey
2026-02-26 16:43   ` Simon Marchi
2026-02-27 13:36     ` Tom Tromey
2026-02-25 21:25 ` [PATCH 2/2] Use enum types for remote fileio flags Tom Tromey
2026-02-26 17:10   ` Simon Marchi
2026-02-27 14:15     ` Tom Tromey [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87jyvyuvbe.fsf@tromey.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox