[PATCH] f2fs-tools: fix an improper #ifdef wrap for Android

[PATCH] f2fs-tools: fix an improper #ifdef wrap for Android

[Park Ju Hyung]

Commit 595fd57a4fd3 ("f2fs-tools: get kernel version via uname(2)")
introduced support for reading kernel version without /proc/version's
presence but improperly wrapped an #ifdef macro for Android, causing
f2fs-tools usage on every Android devices to display
"Info: No support kernel version!".

Fix this by properly wrapping the problematic #ifdef macro.

Also remove 'c.kd = -2' as it will be set to -1 upon open(2) failure
and the rest of the source code isn't doing anything special with
'kd == -2'.

Additionally, show the message when uname(2) also fails from
get_kernel_uname_version().

Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
---
 lib/libf2fs.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 60b84e0..61fc7ec 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -751,23 +751,23 @@ void get_kernel_version(__u8 *version)
 
 void get_kernel_uname_version(__u8 *version)
 {
+	memset(version, 0, VERSION_LEN);
+
 #ifdef HAVE_SYS_UTSNAME_H
 	struct utsname buf;
 
-	memset(version, 0, VERSION_LEN);
-	if (uname(&buf))
-		return;
-
+	if (uname(&buf) == 0) {
 #if !defined(WITH_KERNEL_VERSION)
-	snprintf((char *)version,
-		VERSION_LEN, "%s %s", buf.release, buf.version);
+		snprintf((char *)version,
+			VERSION_LEN, "%s %s", buf.release, buf.version);
 #else
-	snprintf((char *)version,
-		VERSION_LEN, "%s", buf.release);
+		snprintf((char *)version,
+			VERSION_LEN, "%s", buf.release);
 #endif
-#else
-	memset(version, 0, VERSION_LEN);
+	}
 #endif
+
+	MSG(0, "\tInfo: Unable to get the kernel version!\n");
 }
 
 #if defined(__linux__) && defined(_IO) && !defined(BLKGETSIZE)
@@ -845,15 +845,11 @@ int get_device_info(int i)
 		}
 	}
 
-	if (c.kd == -1) {
 #if !defined(WITH_ANDROID) && defined(__linux__)
+	// If this fails, we'll retry with get_kernel_uname_version()
+	if (c.kd == -1)
 		c.kd = open("/proc/version", O_RDONLY);
 #endif
-		if (c.kd < 0) {
-			MSG(0, "\tInfo: No support kernel version!\n");
-			c.kd = -2;
-		}
-	}
 
 	if (c.sparse_mode) {
 		dev->total_sectors = c.device_size / dev->sector_size;
-- 
2.21.0

Re: [PATCH] f2fs-tools: fix an improper #ifdef wrap for Android

[Jaegeuk Kim]

On 04/16, Park Ju Hyung wrote:
> Commit 595fd57a4fd3 ("f2fs-tools: get kernel version via uname(2)")
> introduced support for reading kernel version without /proc/version's
> presence but improperly wrapped an #ifdef macro for Android, causing
> f2fs-tools usage on every Android devices to display
> "Info: No support kernel version!".
> 
> Fix this by properly wrapping the problematic #ifdef macro.
> 
> Also remove 'c.kd = -2' as it will be set to -1 upon open(2) failure
> and the rest of the source code isn't doing anything special with
> 'kd == -2'.
> 
> Additionally, show the message when uname(2) also fails from
> get_kernel_uname_version().
> 
> Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
> ---
>  lib/libf2fs.c | 28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index 60b84e0..61fc7ec 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -751,23 +751,23 @@ void get_kernel_version(__u8 *version)
>  
>  void get_kernel_uname_version(__u8 *version)
>  {
> +	memset(version, 0, VERSION_LEN);
> +
>  #ifdef HAVE_SYS_UTSNAME_H
>  	struct utsname buf;
>  
> -	memset(version, 0, VERSION_LEN);
> -	if (uname(&buf))
> -		return;
> -
> +	if (uname(&buf) == 0) {
>  #if !defined(WITH_KERNEL_VERSION)
> -	snprintf((char *)version,
> -		VERSION_LEN, "%s %s", buf.release, buf.version);
> +		snprintf((char *)version,
> +			VERSION_LEN, "%s %s", buf.release, buf.version);
>  #else
> -	snprintf((char *)version,
> -		VERSION_LEN, "%s", buf.release);
> +		snprintf((char *)version,
> +			VERSION_LEN, "%s", buf.release);
>  #endif
> -#else
> -	memset(version, 0, VERSION_LEN);
> +	}
>  #endif
> +
> +	MSG(0, "\tInfo: Unable to get the kernel version!\n");
>  }
>  
>  #if defined(__linux__) && defined(_IO) && !defined(BLKGETSIZE)
> @@ -845,15 +845,11 @@ int get_device_info(int i)
>  		}
>  	}
>  
> -	if (c.kd == -1) {
>  #if !defined(WITH_ANDROID) && defined(__linux__)
> +	// If this fails, we'll retry with get_kernel_uname_version()
> +	if (c.kd == -1)
>  		c.kd = open("/proc/version", O_RDONLY);
>  #endif
> -		if (c.kd < 0) {
> -			MSG(0, "\tInfo: No support kernel version!\n");
> -			c.kd = -2;

If there are multiple devices, we don't need to get the version redundantly.

> -		}
> -	}
>  
>  	if (c.sparse_mode) {
>  		dev->total_sectors = c.device_size / dev->sector_size;
> -- 
> 2.21.0
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH] f2fs-tools: fix an improper #ifdef wrap for Android

[Ju Hyung Park]

Hi Jaegeuk,

On Wed, Apr 17, 2019 at 5:35 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> > -             if (c.kd < 0) {
> > -                     MSG(0, "\tInfo: No support kernel version!\n");
> > -                     c.kd = -2;
>
> If there are multiple devices, we don't need to get the version redundantly.

Oh, didn't think of that case.

Can we just read the kernel version and store to memory from
initialization of the program?
I see that the code for getting the kernel version is already
duplicated in 2 different files: fsck/mount.c and mkfs/f2fs_format.c

/* get kernel version */
if (c.kd >= 0) {
        dev_read_version(c.version, 0, VERSION_LEN);
        get_kernel_version(c.version);
} else {
        get_kernel_uname_version(c.version);
}

Thanks.

Re: [PATCH] f2fs-tools: fix an improper #ifdef wrap for Android

[Jaegeuk Kim]

Hi Ju Hyung,

On 04/17, Ju Hyung Park wrote:
> Hi Jaegeuk,
> 
> On Wed, Apr 17, 2019 at 5:35 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> > > -             if (c.kd < 0) {
> > > -                     MSG(0, "\tInfo: No support kernel version!\n");
> > > -                     c.kd = -2;
> >
> > If there are multiple devices, we don't need to get the version redundantly.
> 
> Oh, didn't think of that case.
> 
> Can we just read the kernel version and store to memory from
> initialization of the program?
> I see that the code for getting the kernel version is already
> duplicated in 2 different files: fsck/mount.c and mkfs/f2fs_format.c

The intention was to separate mkfs.f2fs from fsck.f2fs, since mkfs.f2fs should
be run as standalone.

> 
> /* get kernel version */
> if (c.kd >= 0) {
>         dev_read_version(c.version, 0, VERSION_LEN);
>         get_kernel_version(c.version);
> } else {
>         get_kernel_uname_version(c.version);
> }
> 
> Thanks.