From owner-cvs-src@FreeBSD.ORG  Thu Mar  4 10:12:16 2004
Return-Path: <owner-cvs-src@FreeBSD.ORG>
Delivered-To: cvs-src@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id 5E5FC16A4CE
	for <cvs-src@FreeBSD.org>; Thu,  4 Mar 2004 10:12:16 -0800 (PST)
Received: from root.org (root.org [67.118.192.226])
	by mx1.FreeBSD.org (Postfix) with SMTP id 3376443D41
	for <cvs-src@FreeBSD.org>; Thu,  4 Mar 2004 10:12:16 -0800 (PST)
	(envelope-from nate@root.org)
Received: (qmail 24628 invoked by uid 1000); 4 Mar 2004 18:12:16 -0000
Date: Thu, 4 Mar 2004 10:12:16 -0800 (PST)
From: Nate Lawson <nate@root.org>
To: Takanori Watanabe <takawata@FreeBSD.org>
In-Reply-To: <20040304170353.64D4F16A4D4@hub.freebsd.org>
Message-ID: <20040304100856.Y24532@root.org>
References: <20040304170353.64D4F16A4D4@hub.freebsd.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
cc: cvs-src@FreeBSD.org
cc: src-committers@FreeBSD.org
cc: cvs-all@FreeBSD.org
Subject: Re: cvs commit: src/usr.sbin/acpi/acpiconf acpiconf.c
X-BeenThere: cvs-src@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: CVS commit messages for the src tree <cvs-src.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-src>,
	<mailto:cvs-src-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/cvs-src>
List-Post: <mailto:cvs-src@freebsd.org>
List-Help: <mailto:cvs-src-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-src>,
	<mailto:cvs-src-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 04 Mar 2004 18:12:16 -0000

On Thu, 4 Mar 2004, Takanori Watanabe wrote:
>   FreeBSD src repository
>
>   Modified files:
>     usr.sbin/acpi/acpiconf acpiconf.c
>   Log:
>   Make unprivilaged user can see battery info.
>
>   Revision  Changes    Path
>   1.13      +5 -2      src/usr.sbin/acpi/acpiconf/acpiconf.c

Hi, welcome back!  A few comments...

> --- src/usr.sbin/acpi/acpiconf/acpiconf.c:1.12	Tue Dec 30 09:28:06 2003
> +++ src/usr.sbin/acpi/acpiconf/acpiconf.c	Thu Mar  4 09:03:49 2004
> @@ -44,13 +44,16 @@
>  #define RC_RESUME_PATH	"/etc/rc.resume"
>
>  static int	acpifd;
> -
>  static int
>  acpi_init()

I think that newline should be left for style(9)

>  {
>  	acpifd = open(ACPIDEV, O_RDWR);
> -	if (acpifd == -1)
> +	if (acpifd == -1){
> +		acpifd = open(ACPIDEV, O_RDONLY);
> +	}
> +	if (acpifd == -1){
>  		err(EX_OSFILE, ACPIDEV);
> +	}
>  }
>
>  static int

I am concerned about this change since being able to open a descriptor
read-only implies they can do ioctl, which means they can also suspend the
system.  If we have to do this, we might have to go the setgid route
although I don't necessarily want to deal with the security implications
of that.  Users can get battery status through apm(8) but not battery
info.

-Nate