This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Add system(NULL) to fileio
- From: Corinna Vinschen <vinschen at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Mon, 12 Jun 2006 14:11:24 +0200
- Subject: Re: Add system(NULL) to fileio
- References: <4489D824.40605@codesourcery.com>
- Reply-to: gdb-patches at sourceware dot org
On Jun 9 21:20, Nathan Sidwell wrote:
> We noticed that system(NULL) has a special meaning. This patch augments the
> fileio protocol to support it.
>
> Fortunately the current protocol's LENGTH field is strlen + 1, so it can never
> legitimately be zero. So I use that to indicate a NULL string is being passed
> -- we don't have to presume a NULL pointer is all bits zero :)
>
> Tested with a modified libgloss for an m68k target. ok?
> [...]
As for the actual code, since I wrote the original fileio stuff, I thought
I could have a look.
> ! if (length)
> ! {
> ! /* Request commandline using 'm' packet */
> ! cmdline = alloca (length);
> ! retlength = remote_read_bytes (ptrval, (gdb_byte *) cmdline, length);
> ! if (retlength != length)
> ! {
> ! remote_fileio_ioerror ();
> ! return;
> ! }
> ! }
> !
> ! /* Check if system(3) has been explicitely allowed using the
> ! `set remote system-call-allowed 1' command. If not, return
> ! EPERM */
Wouldn't it be better to add a short comment here explaining the !length
case, something along the lines of "A zero length indicates a NULL
argument to system(3) ... handled according to POSIX (check if shell
exists) even if the system call hasn't been allowed by the user"?
> ! if (!remote_fio_system_call_allowed)
> {
> ! if (!length)
> ! remote_fileio_return_success (0);
> ! else
> ! remote_fileio_reply (-1, FILEIO_EPERM);
> return;
> }
>
> remote_fio_no_longjmp = 1;
> ret = system (cmdline);
>
> ! if (!length)
> ! remote_fileio_return_success (ret);
> ! else if (ret == -1)
> remote_fileio_return_errno (-1);
> else
> remote_fileio_return_success (WEXITSTATUS (ret));
Everything else looks good to me.
Corinna
--
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat