Re: git: 1808b5577da8 - main - Add efiwake tool

From: Jessica Clarke <jrtc27_at_freebsd.org>
Date: Fri, 28 Apr 2023 02:05:55 UTC
On 28 Apr 2023, at 02:29, Konstantin Belousov <kib@FreeBSD.org> wrote:
> 
> The branch main has been updated by kib:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=1808b5577da8d3fcffc70973afef250f428f9381
> 
> commit 1808b5577da8d3fcffc70973afef250f428f9381
> Author:     Johannes Totz <jo@bruelltuete.com>
> AuthorDate: 2023-04-26 16:16:55 +0000
> Commit:     Konstantin Belousov <kib@FreeBSD.org>
> CommitDate: 2023-04-28 01:28:51 +0000
> 
>    Add efiwake tool

Hi,
This has quite a few style(9) violations. Please fix them or work with
the author to get them fixed.

>    Reviewed by:    kib
>    MFC after:      1 week
>    Differential revision:  https://reviews.freebsd.org/D36714
> ---
> usr.sbin/Makefile          |   2 +-
> usr.sbin/efiwake/Makefile  |  13 +++++
> usr.sbin/efiwake/efiwake.c | 134 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 148 insertions(+), 1 deletion(-)
> 
> diff --git a/usr.sbin/Makefile b/usr.sbin/Makefile
> index 9fa1dc05a1cc..fb707e515eaa 100644
> --- a/usr.sbin/Makefile
> +++ b/usr.sbin/Makefile
> @@ -129,7 +129,7 @@ SUBDIR.${MK_OPENSSL}+=	certctl
> .endif
> SUBDIR.${MK_CXGBETOOL}+=	cxgbetool
> SUBDIR.${MK_DIALOG}+=	bsdconfig
> -SUBDIR.${MK_EFI}+=	efivar efidp efibootmgr efitable
> +SUBDIR.${MK_EFI}+=	efivar efidp efibootmgr efitable efiwake
> .if ${MK_OPENSSL} != "no"
> SUBDIR.${MK_EFI}+=	uefisign
> .endif
> diff --git a/usr.sbin/efiwake/Makefile b/usr.sbin/efiwake/Makefile
> new file mode 100644
> index 000000000000..ed2292f8a3ac
> --- /dev/null
> +++ b/usr.sbin/efiwake/Makefile
> @@ -0,0 +1,13 @@
> +# $FreeBSD$
> +
> +PACKAGE=	efi-tools
> +
> +PROG=	efiwake
> +MAN=
> +
> +SRCS=	efiwake.c
> +
> +EFIBOOT=${SRCTOP}/stand/efi
> +CFLAGS+=-I${EFIBOOT}/include
> +
> +.include <bsd.prog.mk>
> diff --git a/usr.sbin/efiwake/efiwake.c b/usr.sbin/efiwake/efiwake.c
> new file mode 100644
> index 000000000000..67497e2ede03
> --- /dev/null
> +++ b/usr.sbin/efiwake/efiwake.c
> @@ -0,0 +1,134 @@
> +/*-
> + * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
> + *
> + * Copyright (c) 2023 Johannes Totz
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include <err.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <sysexits.h>
> +#include <unistd.h>
> +#include <sys/efiio.h>

Headers don’t follow correct grouping.

> +
> +static void usage(void)

New line after static void needed.

> +{
> +	printf("Usage:\n"

stderr?

> +	    "  efiwake                          -- print out current EFI time and wake time\n"
> +	    "  efiwake -d                       -- disable wake time\n"
> +	    "  efiwake -e yyyy-mm-ddThh:mm:ss   -- enable wake time\n"
> +	);
> +
> +	exit(EX_USAGE);
> +}
> +
> +int main(int argc, char **argv)

New line after int needed.

> +{
> +	struct efi_waketime_ioc	waketime;
> +	int error, ch;
> +	bool enable = false, disable = false;
> +
> +	memset(&waketime, 0, sizeof(waketime));
> +
> +	while ((ch = getopt(argc, argv, "de:")) != -1) {
> +		switch (ch) {
> +		case 'd':
> +			disable = true;
> +			break;
> +		case 'e':
> +			if (sscanf(optarg,
> +			    "%hu-%02hhu-%02hhuT%02hhu:%02hhu:%02hhu",
> +			    &waketime.waketime.tm_year, &waketime.waketime.tm_mon,
> +			    &waketime.waketime.tm_mday, &waketime.waketime.tm_hour,
> +			    &waketime.waketime.tm_min, &waketime.waketime.tm_sec)
> +			    != 6) {
> +				usage();
> +			}
> +			enable = true;
> +			break;
> +		case '?':
> +		default:
> +			usage();
> +		}
> +	}
> +	argc -= optind;
> +	argv += optind;
> +
> +	if (argc > 0)
> +		usage();
> +	if (disable && enable)
> +		usage();
> +
> +	int efi_fd = open("/dev/efi", O_RDWR);

These all need to be at the start of the block.

> +	if (efi_fd < 0)
> +		err(EX_OSERR, "cannot open /dev/efi");
> +
> +	struct efi_tm	now;
> +	error = ioctl(efi_fd, EFIIOC_GET_TIME, &now);
> +	if (error != 0)
> +		err(EX_OSERR, "cannot get EFI time");
> +
> +	/* EFI's time can be different from kernel's time. */
> +	printf("Current EFI time: %u-%02u-%02uT%02u:%02u:%02u\n",
> +	    now.tm_year, now.tm_mon, now.tm_mday, now.tm_hour, now.tm_min,
> +	    now.tm_sec);
> +
> +	if (disable) {
> +		/* It's tempting to preserve the current timer value.

/* needs to be on its own line.

Regards,
Jess

> +		 * However, wonky EFI implementations sometimes return bogus
> +		 * dates for the wake timer and would then fail disabling it
> +		 * here.
> +		 * A safe timer value is the current EFI time.
> +		 */
> +		waketime.waketime = now;
> +		waketime.enabled = 0;
> +		error = ioctl(efi_fd, EFIIOC_SET_WAKETIME, &waketime);
> +		if (error != 0)
> +			err(EX_OSERR, "cannot disable EFI wake time");
> +	}
> +	if (enable) {
> +		waketime.enabled = 1;
> +		error = ioctl(efi_fd, EFIIOC_SET_WAKETIME, &waketime);
> +		if (error != 0)
> +			err(EX_OSERR, "cannot enable EFI wake time");
> +	}
> +
> +	/* Confirm to user what the wake time has been set to. */
> +	error = ioctl(efi_fd, EFIIOC_GET_WAKETIME, &waketime);
> +	if (error != 0)
> +		err(EX_OSERR, "cannot get EFI wake time");
> +
> +	printf("EFI wake time: %u-%02u-%02uT%02u:%02u:%02u; enabled=%i, pending=%i\n",
> +	    waketime.waketime.tm_year, waketime.waketime.tm_mon,
> +	    waketime.waketime.tm_mday, waketime.waketime.tm_hour,
> +	    waketime.waketime.tm_min, waketime.waketime.tm_sec,
> +	    waketime.enabled, waketime.pending);
> +
> +	close(efi_fd);
> +	return (0);
> +}