From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11691 invoked by alias); 16 Jan 2009 09:07:17 -0000 Received: (qmail 11682 invoked by uid 22791); 16 Jan 2009 09:07:16 -0000 X-SWARE-Spam-Status: No, hits=-0.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_65,J_CHICKENPOX_74,RCVD_IN_SORBS_WEB,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout5.012.net.il (HELO mtaout5.012.net.il) (84.95.2.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 16 Jan 2009 09:06:36 +0000 Received: from conversion-daemon.i_mtaout5.012.net.il by i_mtaout5.012.net.il (HyperSendmail v2004.12) id <0KDK001003JK2C00@i_mtaout5.012.net.il> for gdb-patches@sourceware.org; Fri, 16 Jan 2009 11:06:43 +0200 (IST) Received: from HOME-C4E4A596F7 ([77.127.202.36]) by i_mtaout5.012.net.il (HyperSendmail v2004.12) with ESMTPA id <0KDK00FT83YXPSB0@i_mtaout5.012.net.il>; Fri, 16 Jan 2009 11:06:43 +0200 (IST) Date: Fri, 16 Jan 2009 09:07:00 -0000 From: Eli Zaretskii Subject: Re: Use external editor in 'commands' command In-reply-to: To: Alfredo Ortega Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: References: X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-01/txt/msg00375.txt.bz2 > Date: Fri, 16 Jan 2009 05:38:41 -0200 > From: Alfredo Ortega > > Thanks for the corrections. Here are are both changelogs and the > updated diff, I hope there are less errors now... > I promise that my next patches will be better! No one is born with this, so there's nothing wrong in making such minor mistakes. I have several comments to your code: > + char vitmp[50]; vitmp[] is a file name, so 50 is not nearly enough characters to hold the longest possible name. At the very least, please use FILENAME_MAX, or (better) some dynamic code that grows it as needed, because some hosts (e.g., Hurd) don't have any limitations on file name length. > + char cmdline[100]; Same here: 100 is not enough, because $EDITOR holds a file name. > + if (!strcmp(COMMANDS_EDCOMMAND,p)) { This is not GNU style of laying out brace-delimited blocks; please use the same style and indentation as elsewhere in GDB sources. > + strcpy(vitmp,"/tmp/.gdbXXXXXX"); Please leave a blank between the function name and the left parenthesis, and also between the comma and the following argument in argument lists. Like this: strcpy (vitmp, "/tmp/.gdbXXXXXX"); Btw, using "/tmp/.gdbXXXXXX" is non-portable to Windows, where there's no guarantee there will be a "/tmp" on the current drive, and it will not work at all in the DJGPP (a.k.a. DOS) port of GDB, because DOS filesystems do not allow file names with a leading dot. > + /* Generates the temporal file name*/ ^^^^^^^^ "temporary" > + /* vitmp = tempnam(NULL,".gdb"); this is more secure according to man mkstemp, but gcc complains... */ What complaint do you see? > + if (mkstemp(vitmp)<0) return; Are you using mkstemp from libiberty? Because otherwise it may not be available on the host. > + if (fsizeline)+1) { > + fclose(tmpstream); > + unlink(vitmp); > + return; > + }; Don't we want some error message in this case? > + if ((editor = (char *) getenv ("EDITOR")) == NULL) > + editor = "/bin/ex"; This is likewise non-portable: "/bin/ex" is only guaranteed to exist on Posix platforms.