From owner-freebsd-hackers@FreeBSD.ORG Sun Jan 2 23:47:08 2011 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 058C3106564A for ; Sun, 2 Jan 2011 23:47:08 +0000 (UTC) (envelope-from lists@eitanadler.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id CC4378FC16 for ; Sun, 2 Jan 2011 23:47:07 +0000 (UTC) Received: by iwn39 with SMTP id 39so13244239iwn.13 for ; Sun, 02 Jan 2011 15:47:07 -0800 (PST) Received: by 10.231.36.205 with SMTP id u13mr3651052ibd.155.1294012027313; Sun, 02 Jan 2011 15:47:07 -0800 (PST) MIME-Version: 1.0 Received: by 10.231.178.195 with HTTP; Sun, 2 Jan 2011 15:46:47 -0800 (PST) In-Reply-To: <20110102101845.GC90883@deviant.kiev.zoral.com.ua> References: <6BEFA669-2E1F-44E6-897A-0A51DA939A74@gmail.com> <20110102101845.GC90883@deviant.kiev.zoral.com.ua> From: Eitan Adler Date: Sun, 2 Jan 2011 18:46:47 -0500 Message-ID: To: Kostik Belousov Content-Type: text/plain; charset=UTF-8 Cc: hackers@freebsd.org Subject: Re: [patch] have rtprio check that arguments are numeric; change atoi to strtol X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Jan 2011 23:47:08 -0000 What about this patch? I incorporated your feedback so I am not going to reply inline. > The syntax of the prio commands is weird, there is an obvious corner > (or wrongly handled) case, where the command name starts with a digit. > Command name starting with dash is even harder. > I agree - and I wouldn't mind seeing the syntax changed (along with the licensed changed to a 2 clause BSD license) - but I suspect the benefits conferred by those two things would not be enough to overcome the reluctance to change a very old command (since 1994). If I'm wrong I'll gladly write a "cleanroom" version with sane syntax. Index: rtprio.c =================================================================== --- rtprio.c (revision 216679) +++ rtprio.c (working copy) @@ -41,6 +41,7 @@ #include #include +#include #include #include #include @@ -54,14 +55,12 @@ char **argv; { char *p; - int proc = 0; + pid_t proc; struct rtprio rtp; + char *invalidchar; - /* find basename */ - if ((p = rindex(argv[0], '/')) == NULL) - p = argv[0]; - else - ++p; + proc = 0; + p = basename(argv[0]); if (!strcmp(p, "rtprio")) rtp.type = RTP_PRIO_REALTIME; @@ -70,8 +69,10 @@ switch (argc) { case 2: - proc = abs(atoi(argv[1])); /* Should check if numeric - * arg! */ + proc = (int)strtol(argv[1], &invalidchar, 10); + if (*invalidchar != '\0') + errx(1,"Process should be a pid"); + proc = abs(proc); /* FALLTHROUGH */ case 1: if (rtprio(RTP_LOOKUP, proc, &rtp) != 0) @@ -104,16 +105,20 @@ break; } } else { - rtp.prio = atoi(argv[1]); + rtp.prio = (int)strtol(argv[1], &invalidchar, 10); + if (*invalidchar != '\0') + errx(1,"Priority should be a number", invalidchar); } } else { usage(); break; } - if (argv[2][0] == '-') - proc = -atoi(argv[2]); - + if (argv[2][0] == '-') { + proc = (int)strtol(argv[2]+1, &invalidchar, 10); + if (*invalidchar != '\0') + errx(1,"Process should be a pid"); + } if (rtprio(RTP_SET, proc, &rtp) != 0) err(1, "%s", argv[0]); -- Eitan Adler