From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21137 invoked by alias); 12 Jun 2006 12:11:29 -0000 Received: (qmail 21129 invoked by uid 22791); 12 Jun 2006 12:11:28 -0000 X-Spam-Check-By: sourceware.org Received: from aquarius.hirmke.de (HELO calimero.vinschen.de) (217.91.18.234) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Mon, 12 Jun 2006 12:11:26 +0000 Received: by calimero.vinschen.de (Postfix, from userid 500) id 43F80544008; Mon, 12 Jun 2006 14:11:24 +0200 (CEST) Date: Mon, 12 Jun 2006 12:11:00 -0000 From: Corinna Vinschen To: gdb-patches@sourceware.org Subject: Re: Add system(NULL) to fileio Message-ID: <20060612121124.GA19317@calimero.vinschen.de> Reply-To: gdb-patches@sourceware.org Mail-Followup-To: gdb-patches@sourceware.org References: <4489D824.40605@codesourcery.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4489D824.40605@codesourcery.com> User-Agent: Mutt/1.4.2i X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-06/txt/msg00150.txt.bz2 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