Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 09 Dec 2005 14:00:54 +0100
From:      des@des.no (=?iso-8859-1?q?Dag-Erling_Sm=F8rgrav?=)
To:        Nate Lawson <nate@root.org>
Cc:        njl@freebsd.org, Fredrik Lindberg <fli+freebsd-current@shapeshifter.se>, Travis Mikalson <bofh@terranova.net>, current@freebsd.org
Subject:   Re: powerd
Message-ID:  <864q5is9vd.fsf@xps.des.no>
In-Reply-To: <43997CEC.1070607@root.org> (Nate Lawson's message of "Fri, 09 Dec 2005 21:47:40 %2B0900")
References:  <43938F61.1050202@terranova.net> <4393F60E.2040106@shapeshifter.se> <86mzjflc97.fsf@xps.des.no> <439495B1.5060305@shapeshifter.se> <861x0qmuen.fsf@xps.des.no> <43956ADF.4050504@shapeshifter.se> <86slt6lb9s.fsf@xps.des.no> <4395A265.8080006@root.org> <86d5k9eric.fsf@xps.des.no> <43997CEC.1070607@root.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Nate Lawson <nate@root.org> writes:
> Dag-Erling Sm=F8rgrav wrote:
>> +static enum {
>> +	ac_none,
>> +	ac_acpi_sysctl,
>> +	ac_acpi_devd,
>> +#ifdef USE_APM
>> +	ac_apm,
>> +#endif
>> +} acline_mode;
>
> Prefer enums be all CAPS, seems like ac_apm can be left in the enum
> without an #ifdef because it's only checked by code in an #ifdef below.

Having the #ifdef here allows the compiler to warn us if we forget it
somewhere else.

>> +#ifdef __i386__
>> +	} else if ((apm_fd =3D open(APMDEV, O_RDONLY)) >=3D 0) {
>> +		if (vflag)
>> +			warnx("using APM for AC line status");
>> +		acline_mode =3D ac_apm;
>> +#endif
>
> Don't you want to use your new USE_APM define here?

Yes, that was an oversight.

>>  -static int
>> -acline_read()
>> +static void
>> +acline_read(void)
>
> Is this correct?  I thought only the prototype (above) should have
> void as an arg.

Yes, and no.

>>   static void
>>  devd_close(void)
>>  {
>>  -	if (devd_pipe < 0)
>> -		return;
>> -
>> -	pthread_kill(devd_thread, SIGTERM);
>>  	close(devd_pipe);
>> -}
>
> Seems like this function can go away now and we can just conditionally
> close devd_pipe (if !=3D -1) at the end of main().

actually, devd_close() should include the line 'devd_pipe =3D -1;' and
all instances of close(devd_pipe) should be replaced with
devd_close().

>> -	/*
>> -	 * Exit cleanly on signals; devd may send a SIGPIPE if it dies.  We
>> -	 * do this before acline_init() since it may create a thread and we
>> -	 * want it to inherit our signal mask.
>> -	 */
>> -	signal(SIGINT, handle_sigs);
>> -	signal(SIGTERM, handle_sigs);
>> -	signal(SIGPIPE, SIG_IGN);
>> -
>
> Don't we still need SIGPIPE handling if reading from devd?

des@xps ~% man signal | grep SIGPIPE
     13    SIGPIPE      terminate process    write on a pipe with no reader

DES
--=20
Dag-Erling Sm=F8rgrav - des@des.no




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?864q5is9vd.fsf>