Re: [PATCH 09/13] rtcwake: do not overwrite device name

[Karel Zak <kzak@redhat.com>]

On Sun, Feb 22, 2015 at 02:42:15PM +0000, Sami Kerola wrote:
> This improves error messaging, and allows freeing path to device when it
> is not needed.
> 
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  sys-utils/rtcwake.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
> index f951c9c..7f1e351 100644
> --- a/sys-utils/rtcwake.c
> +++ b/sys-utils/rtcwake.c
> @@ -49,7 +49,7 @@
>  
>  #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"

This is really bad name, what about SYS_WAKEUP_PATH_TEMPLATE ?

> +static int open_dev_rtc(const char *devname)
> +{
> +	int fd;
> +	char *devpath;
> +	size_t len = strlen(devname);
> +
> +	devpath = xmalloc(sizeof "/dev/" + len);
> +	memcpy(devpath, "/dev/", sizeof "/dev/");
> +	memcpy(devpath + strlen("/dev/"), devname, len + 1);

 if (startswith(devname, "/dev"))
    devpath = devname;
 else
    xasprintf(&devpath, "/dev/%s", devname);  

> +#ifdef O_CLOEXEC
> +	fd = open(devpath, O_RDONLY | O_CLOEXEC);
> +#else
> +	fd = open(devpath, O_RDONLY);
> +#endif

 we use O_CLOEXEC everywhere, #ifdef is unecessary

> +	if (fd < 0)
> +		err(EXIT_FAILURE, _("%s: unable to find device"), devpath);

 if (devname != devpath)
    free(devpath);

> +	free(devpath);
> +	return fd;
> +}

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com