On Wed, Apr 07 2021, Klemens Nanni <[email protected]> wrote:
> On Tue, Apr 06, 2021 at 11:35:44PM +0200, Jeremie Courreges-Anglas wrote:
[...]
>> > @@ -321,40 +328,49 @@ handle_client(int sock_fd, int ctl_fd)
>> > return reply.newstate;
>> > }
>> >
>> > -void
>> > +int
>> > suspend(int ctl_fd)
>> > {
>> > + int ret;
>> > +
>> > logmsg(LOG_NOTICE, "system suspending");
>> > power_status(ctl_fd, 1, NULL);
>> > do_etc_file(_PATH_APM_ETC_SUSPEND);
>> > sync();
>> > sleep(1);
>> > - if (ioctl(ctl_fd, APM_IOC_SUSPEND, 0) == -1)
>> > + if ((ret = ioctl(ctl_fd, APM_IOC_SUSPEND, 0)) == -1)
>> > logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
>>
>> If you hit the error case logmsg() might clobber the errno and make it
>> unavailable later...
> Right, better use a "save_errno" in all three functions, then.
And those functions could return zero/errno, this saves a few more
lines. ok?
BTW now handle_client() could stop setting newstate on error. But as far as
I'm concerned that's for another day and another diff.
Index: apmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.104
diff -u -p -r1.104 apmd.c
--- apmd.c 6 Apr 2021 22:12:48 -0000 1.104
+++ apmd.c 6 Apr 2021 22:27:26 -0000
@@ -271,18 +271,15 @@ handle_client(int sock_fd, int ctl_fd)
switch (cmd.action) {
case SUSPEND:
reply.newstate = SUSPENDING;
- if (suspend(ctl_fd) == -1)
- reply.error = errno;
+ reply.error = suspend(ctl_fd);
break;
case STANDBY:
reply.newstate = STANDING_BY;
- if (stand_by(ctl_fd) == -1)
- reply.error = errno;
+ reply.error = stand_by(ctl_fd);
break;
case HIBERNATE:
reply.newstate = HIBERNATING;
- if (hibernate(ctl_fd) == -1)
- reply.error = errno;
+ reply.error = hibernate(ctl_fd);
break;
case SETPERF_LOW:
reply.newstate = NORMAL;
@@ -329,46 +326,58 @@ handle_client(int sock_fd, int ctl_fd)
int
suspend(int ctl_fd)
{
- int ret;
+ int error = 0;
logmsg(LOG_NOTICE, "system suspending");
power_status(ctl_fd, 1, NULL);
do_etc_file(_PATH_APM_ETC_SUSPEND);
sync();
sleep(1);
- if ((ret = ioctl(ctl_fd, APM_IOC_SUSPEND, 0)) == -1)
+
+ if (ioctl(ctl_fd, APM_IOC_SUSPEND, 0) == -1) {
+ error = errno;
logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
- return (ret);
+ }
+
+ return error;
}
int
stand_by(int ctl_fd)
{
- int ret;
+ int error = 0;
logmsg(LOG_NOTICE, "system entering standby");
power_status(ctl_fd, 1, NULL);
do_etc_file(_PATH_APM_ETC_STANDBY);
sync();
sleep(1);
- if ((ret = ioctl(ctl_fd, APM_IOC_STANDBY, 0)) == -1)
+
+ if (ioctl(ctl_fd, APM_IOC_STANDBY, 0) == -1) {
+ error = errno;
logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
- return (ret);
+ }
+
+ return error;
}
int
hibernate(int ctl_fd)
{
- int ret;
+ int error = 0;
logmsg(LOG_NOTICE, "system hibernating");
power_status(ctl_fd, 1, NULL);
do_etc_file(_PATH_APM_ETC_HIBERNATE);
sync();
sleep(1);
- if ((ret = ioctl(ctl_fd, APM_IOC_HIBERNATE, 0)) == -1)
+
+ if (ioctl(ctl_fd, APM_IOC_HIBERNATE, 0) == -1) {
+ error = errno;
logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
- return (ret);
+ }
+
+ return error;
}
void
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
