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ørgrav 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 = open(APMDEV, O_RDONLY)) >= 0) {
>> + if (vflag)
>> + warnx("using APM for AC line status");
>> + acline_mode = 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 != -1) at the end of main().
actually, devd_close() should include the line 'devd_pipe = -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
--
Dag-Erling Smørgrav - des@des.no
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?864q5is9vd.fsf>