Re: [PATCH 2/4] lsmem: new tool

[Heiko Carstens <heiko.carstens@de.ibm.com>]

On Fri, Nov 04, 2016 at 11:39:31AM +0100, Karel Zak wrote:
> > However it's of course fine with me to make the summary lines optional.
> 
> I have added --summary[=never,always,only]
> 
>     never  - disable summary lines at all (forced for parsable  outputs)
>     always - default for standard output
>     only   - prints only summary, no table with blocks, default when
>              --summary specified without argument
> 
> > Also fine with me :) Thank you for taking care of this!
> 
>     https://github.com/karelzak/util-linux/tree/mem-tools
> 
> This branch contains the new lsmem; the next week I'll cleanup chmem 
> and merge it to the master branch (after v2.29 release, probably
> Monday).

Looks all good to me. Just two comments:

- you changed the alignment of nearly all columns from left to right,
except for STATE and RANGE. For RANGE it does make sense to keep it aligned
to the left, however for STATE it looks a bit inconsistent. Not sure if you
missed to make it also align to the right?

- the updated man page now says that the default output is subject to
change; but it should not change in order to be compatible with the old
lsmem tool within s390-tools.

Also you might consider applying the simple patch below ;)

>From 270714b84caae0977e12afd7f59f88e051463e66 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Fri, 4 Nov 2016 12:56:08 +0100
Subject: [PATCH] lsmem: improve node lookup

Break the loop as soon as we found the node a memory block belongs to,
it doesn't make sense to continue scanning.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 sys-utils/lsmem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sys-utils/lsmem.c b/sys-utils/lsmem.c
index 1dcf8a8e405d..10afc3cb6c8c 100644
--- a/sys-utils/lsmem.c
+++ b/sys-utils/lsmem.c
@@ -265,6 +265,7 @@ static int memory_block_get_node(char *name)
 		if (!isdigit_string(de->d_name + 4))
 			continue;
 		node = strtol(de->d_name + 4, NULL, 10);
+		break;
 	}
 	closedir(dir);
 	return node;
-- 
2.8.4