Re: [PATCH 05/13] erofs-utils: lib: keep self maintained devname

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2023/8/14 11:42, Jingbo Xu wrote:
> Keep self allocated and maintained devname in erofs_sb_info.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> ---
>   include/erofs/internal.h |  2 +-
>   lib/io.c                 | 14 ++++++++++++--
>   2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index a04e6a6..892dc96 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -61,7 +61,7 @@ struct erofs_device_info {
>   
>   struct erofs_sb_info {
>   	struct erofs_device_info *devs;
> -	const char *devname;
> +	char *devname;
>   
>   	u64 total_blocks;
>   	u64 primarydevice_blocks;
> diff --git a/lib/io.c b/lib/io.c
> index 8d84de2..1545436 100644
> --- a/lib/io.c
> +++ b/lib/io.c
> @@ -10,6 +10,7 @@
>   #ifndef _GNU_SOURCE
>   #define _GNU_SOURCE
>   #endif
> +#include <stdlib.h>
>   #include <sys/stat.h>
>   #include <sys/ioctl.h>
>   #include "erofs/io.h"
> @@ -46,6 +47,7 @@ static int dev_get_blkdev_size(int fd, u64 *bytes)
>   void dev_close(struct erofs_sb_info *sbi)
>   {
>   	close(sbi->devfd);
> +	free(sbi->devname);
>   	sbi->devname = NULL;
>   	sbi->devfd   = -1;
>   	sbi->devsz   = 0;
> @@ -95,7 +97,11 @@ int dev_open(struct erofs_sb_info *sbi, const char *dev)
>   		return -EINVAL;
>   	}
>   
> -	sbi->devname = dev;
> +	sbi->devname = strdup(dev);

Could we move sbi->devname assignment to the beginning of the function?
e.g.

	..
         sbi->devname = strdup(dev);
         if (!sbi->devname)
                 return -ENOMEM;

         fd = open(dev, O_RDWR | O_CREAT | O_BINARY, 0644);
	...


> +	if (!sbi->devname) {
> +		close(fd);
> +		return -ENOMEM;
> +	}
>   	sbi->devfd = fd;
>   
>   	erofs_info("successfully to open %s", dev);
> @@ -136,8 +142,12 @@ int dev_open_ro(struct erofs_sb_info *sbi, const char *dev)
>   		return -errno;
>   	}
>   
> +	sbi->devname = strdup(dev);
> +	if (!sbi->devname) {
> +		close(fd);
> +		return -ENOMEM;
> +	}

Same here.

Thanks,
Gao Xiang