From owner-freebsd-bugs@FreeBSD.ORG Mon Jul 21 00:14:09 2008 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BC104106567A for ; Mon, 21 Jul 2008 00:14:09 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from fg-out-1718.google.com (fg-out-1718.google.com [72.14.220.159]) by mx1.freebsd.org (Postfix) with ESMTP id 47AFB8FC14 for ; Mon, 21 Jul 2008 00:14:08 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: by fg-out-1718.google.com with SMTP id l26so748384fgb.35 for ; Sun, 20 Jul 2008 17:14:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=vpSwim4EPAG2ARIFS4ZE+F9bNhZrC8VIVFZx1NvEmwA=; b=IYbMjPY264U4i2ZNLzd++sJTjL77afcDig6lLFgm442jZkxmO3V2X/KQ2t6IVdFrk0 q4naU57WcvTEfXDnpe4X0SLJJmGN5Brx3rPFDN494DdkGJLpyfxqcEiTIZCRDBQUZvAZ SdpUcV4lpooNq2neQ/mZnRL1fpYJAZe0ADnf0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=qjX+x5ULdovmKEbwVwEA0cNleL1OyrvAc7vwjsDWFpVjVrwpzJqzdFCLlhop7+HRk0 oIZI90dy2aqZVmSNeO6pgAmecxgW1wER1RVoQSz/MjrcbS8rhkbjo3tjlBjDjszT8T52 M1JdlvhRrYQHSojhwUxnlU1tTUAxtNVEsSDVI= Received: by 10.86.23.17 with SMTP id 17mr4037034fgw.44.1216599247665; Sun, 20 Jul 2008 17:14:07 -0700 (PDT) Received: by 10.86.51.1 with HTTP; Sun, 20 Jul 2008 17:14:07 -0700 (PDT) Message-ID: <7d6fde3d0807201714g49eb4a80ncfcc1cc800ad595e@mail.gmail.com> Date: Sun, 20 Jul 2008 17:14:07 -0700 From: "Garrett Cooper" To: "Antoine Brodin" In-Reply-To: <200807202210.m6KMA4cm032331@freefall.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200807202210.m6KMA4cm032331@freefall.freebsd.org> Cc: freebsd-bugs@freebsd.org Subject: Re: bin/125680: atacontrol(8): atacontrol depends on executable in /usr X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jul 2008 00:14:09 -0000 On Sun, Jul 20, 2008 at 3:10 PM, Antoine Brodin wrote: > The following reply was made to PR bin/125680; it has been noted by GNATS. > > From: Antoine Brodin > To: bug-followup@FreeBSD.org, stef@memberwebs.com > Cc: > Subject: Re: bin/125680: atacontrol(8): atacontrol depends on executable in > /usr > Date: Sun, 20 Jul 2008 23:37:13 +0200 > > This is a multi-part message in MIME format. > > --Multipart=_Sun__20_Jul_2008_23_37_13_+0200_SBQJ0Ag63L4yRB6c > Content-Type: text/plain; charset=US-ASCII > Content-Transfer-Encoding: 7bit > > Could you try the attached patch ? > > Cheers, > > Antoine > > --Multipart=_Sun__20_Jul_2008_23_37_13_+0200_SBQJ0Ag63L4yRB6c > Content-Type: text/x-diff; > name="atacontrol.diff" > Content-Disposition: attachment; > filename="atacontrol.diff" > Content-Transfer-Encoding: 7bit > > Index: sbin/atacontrol/atacontrol.c > =================================================================== > RCS file: /home/ncvs/src/sbin/atacontrol/atacontrol.c,v > retrieving revision 1.48 > diff -u -r1.48 atacontrol.c > --- sbin/atacontrol/atacontrol.c 15 May 2008 01:25:29 -0000 1.48 > +++ sbin/atacontrol/atacontrol.c 20 Jul 2008 21:18:35 -0000 > @@ -37,6 +37,7 @@ > #include > #include > #include > +#include > > static const char * > mode2str(int mode) > @@ -517,12 +518,29 @@ > if (ioctl(fd, IOCATARAIDREBUILD, &array) < 0) > warn("ioctl(IOCATARAIDREBUILD)"); > else { > - char buffer[128]; > - sprintf(buffer, "/usr/bin/nice -n 20 /bin/dd " > - "if=/dev/ar%d of=/dev/null bs=1m &", > - array); > - if (system(buffer)) > - warn("background dd"); > + char device[64]; > + char *buffer; > + int arfd; > + > + switch (fork()) { > + case -1: > + err(1, "fork"); > + case 0: > + nice(20); > + snprintf(device, sizeof(device), "/dev/ar%d", > + array); > + if ((arfd = open(device, O_RDONLY)) == -1) > + err(1, "open %s", device); > + if ((buffer = malloc(1024 * 1024)) == NULL) > + err(1, "malloc"); > + while (read(arfd, buffer, 1024 * 1024) > 0) > + ; > + free(buffer); > + close(arfd); > + break; > + default: > + break; > + } > } > exit(EX_OK); > } Good catch. C apps shouldn't depend on other commands' existence for security and performance reasons. A few comments: 1. Why is forking another process necessary? In your patch above you're forcing the parent to do the work while the child goes and dies, so there really isn't any benefit to forking at all other than just exercise fork a bit. 2. If you're going to fork another process, wouldn't it be wiser to waitpid(2) until the child's done? Thanks, -Garrett