[PATCH 00/14] pull: some minor fixes

[PATCH 00/14] pull: some minor fixes

[Sami Kerola]

Hi all,

Here is bunch of small fixes some more significant than other.  Many of
the code changes are outcome of reading cppcheck(1) analysis, which is
not attributed in commit messages.  Perhaps that is ok, if not I'll add
something to the text about the fact when requested so.

The fixes can be also pulled from my github branch minor-fixes, find the
details from bottom of this mail if you prefer using remote repository.


Sami Kerola (14):
  textual: fix some typos
  cal: remove unnused structure and definition
  setterm: remove unnecessary variable
  lscpu: avoid use of bzero() in favor of memset()
  cfdisk: add braces to ensure operation order
  fdformat: match variable and print format types
  mkfs.cramfs: use defined failure name rather than magic value
  fdisk: avoid code duplication
  dmesg: avoid unnecessary variable assignment
  setpriv: avoid alloca() use xmalloc() instead
  whereis: avoid printing uninitialized string
  libfdisk: do not do the same thing twice in single if statement
  libsmartcols: avoid variable dereference after release
  lscpu: avoid double free

 disk-utils/cfdisk.c        | 2 +-
 disk-utils/fdformat.c      | 4 ++--
 disk-utils/fdisk.c         | 9 ++++-----
 disk-utils/mkfs.cramfs.c   | 6 ++----
 lib/strutils.c             | 2 +-
 libfdisk/src/dos.c         | 2 +-
 libfdisk/src/partition.c   | 2 +-
 libmount/src/tab.c         | 2 +-
 libsmartcols/src/line.c    | 2 +-
 libsmartcols/src/version.c | 2 +-
 login-utils/last.c         | 2 +-
 misc-utils/cal.c           | 5 -----
 misc-utils/whereis.c       | 3 +--
 sys-utils/dmesg.c          | 3 ++-
 sys-utils/fstrim.c         | 2 +-
 sys-utils/lscpu-dmi.c      | 2 +-
 sys-utils/lscpu.c          | 2 +-
 sys-utils/setpriv.c        | 4 +++-
 term-utils/setterm.c       | 3 +--
 19 files changed, 26 insertions(+), 33 deletions(-)

The following changes since commit 8b9d1474a4bef66ca0c05a7ebd2341bb02016e24:

  build-sys: release++ (v2.25-rc2) (2014-07-02 11:40:19 +0200)

are available in the git repository at:

  git://github.com/kerolasa/lelux-utiliteetit.git 

for you to fetch changes up to 4e6604e51a5460653cd1189c51f5935ae0cde550:

  lscpu: avoid double free (2014-07-13 18:35:39 +0100)

-- 
2.0.1

[PATCH 01/14] textual: fix some typos

[Sami Kerola]

Found with misspell-check version 2.0d.

Reference: https://github.com/lyda/misspell-check
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 lib/strutils.c             | 2 +-
 libfdisk/src/partition.c   | 2 +-
 libmount/src/tab.c         | 2 +-
 libsmartcols/src/version.c | 2 +-
 login-utils/last.c         | 2 +-
 sys-utils/fstrim.c         | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/strutils.c b/lib/strutils.c
index 31bf9e2..9fe9481 100644
--- a/lib/strutils.c
+++ b/lib/strutils.c
@@ -47,7 +47,7 @@ static int do_scale_by_power (uintmax_t *x, int base, int power)
  * The optinal 'power' variable returns number associated with used suffix
  * {K,M,G,T,P,E,Z,Y}  = {1,2,3,4,5,6,7,8}.
  *
- * The funtion also supports decimal point, for example:
+ * The function also supports decimal point, for example:
  *              0.5MB   = 500000
  *              0.5MiB  = 512000
  *
diff --git a/libfdisk/src/partition.c b/libfdisk/src/partition.c
index 70f707c..32473a3 100644
--- a/libfdisk/src/partition.c
+++ b/libfdisk/src/partition.c
@@ -303,7 +303,7 @@ int fdisk_partition_next_partno(
  *
  * Returns info about partition converted to printable string.
  *
- * For exmaple
+ * For example
  *
  *      struct fdisk_parition *pa;
  *
diff --git a/libmount/src/tab.c b/libmount/src/tab.c
index fd6a7d8..4094fb8 100644
--- a/libmount/src/tab.c
+++ b/libmount/src/tab.c
@@ -390,7 +390,7 @@ int mnt_table_append_trailing_comment(struct libmnt_table *tb, const char *comm)
  * same cache between more threads -- currently the cache does not provide any
  * locking method.
  *
- * This function increments cache refrence counter. It's recomented to use
+ * This function increments cache reference counter. It's recomented to use
  * mnt_unref_cache() after mnt_table_set_cache() if you want to keep the cache
  * referenced by @tb only.
  *
diff --git a/libsmartcols/src/version.c b/libsmartcols/src/version.c
index 9aa4e33..e592ccc 100644
--- a/libsmartcols/src/version.c
+++ b/libsmartcols/src/version.c
@@ -11,7 +11,7 @@
  * @title: Version functions
  * @short_description: functions to get the library version.
  *
- * Note that library version is not the same thing as SONAME verison. The
+ * Note that library version is not the same thing as SONAME version. The
  * libsmarcols uses symbols versioning and SONAME is not modified for releases.
  *
  * The library version and symbols version follow util-linux package versioning.
diff --git a/login-utils/last.c b/login-utils/last.c
index ba58c44..4c70eaa 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -401,7 +401,7 @@ static int list(const struct last_control *ctl, struct utmp *p, time_t t, int wh
 		utline[4] = 0;
 
 	/*
-	 *	Is this something we wanna show?
+	 *	Is this something we want to show?
 	 */
 	if (ctl->show) {
 		char **walk;
diff --git a/sys-utils/fstrim.c b/sys-utils/fstrim.c
index c51c7a8..d52d831 100644
--- a/sys-utils/fstrim.c
+++ b/sys-utils/fstrim.c
@@ -118,7 +118,7 @@ static int has_discard(const char *devname, struct sysfs_cxt *wholedisk)
 	 * This is tricky to read the info from sys/, because the queue
 	 * atrributes are provided for whole devices (disk) only. We're trying
 	 * to reuse the whole-disk sysfs context to optimize this stuff (as
-	 * system usualy have just one disk only).
+	 * system usually have just one disk only).
 	 */
 	if (sysfs_devno_to_wholedisk(dev, NULL, 0, &disk) || !disk)
 		return 1;
-- 
2.0.1

[PATCH 02/14] cal: remove unnused structure and definition

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/cal.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/misc-utils/cal.c b/misc-utils/cal.c
index 1004266..a11136a 100644
--- a/misc-utils/cal.c
+++ b/misc-utils/cal.c
@@ -187,12 +187,7 @@ enum {
 
 #define TODAY_FLAG		0x400		/* flag day for highlighting */
 
-#define FMT_ST_LINES 9
 #define FMT_ST_CHARS 300	/* 90 suffices in most locales */
-struct fmt_st
-{
-	char s[FMT_ST_LINES][FMT_ST_CHARS];
-};
 
 static const int days_in_month[2][13] = {
 	{0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31},
-- 
2.0.1

[PATCH 03/14] setterm: remove unnecessary variable

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 term-utils/setterm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/term-utils/setterm.c b/term-utils/setterm.c
index afbd274..f21466e 100644
--- a/term-utils/setterm.c
+++ b/term-utils/setterm.c
@@ -770,12 +770,11 @@ static void screendump(struct setterm_control *ctl)
 	size_t i, j;
 	ssize_t rc;
 	char *inbuf, *outbuf, *p, *q;
-	char dump_default[] = "screen.dump";
 
 	/* open source and destination files */
 	fd = open_snapshot_device(ctl);
 	if (!ctl->opt_sn_name)
-		ctl->opt_sn_name = dump_default;
+		ctl->opt_sn_name = "screen.dump";
 	out = fopen(ctl->opt_sn_name, ctl->opt_snap ? "w" : "a");
 	if (!out)
 		err(EXIT_DUMPFILE, _("can not open dump file %s for output"), ctl->opt_sn_name);
-- 
2.0.1

[PATCH 04/14] lscpu: avoid use of bzero() in favor of memset()

[Sami Kerola]

The bzero() is marked as LEGACY in POSIX.1-2001.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/lscpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
index b64c1f9..062dff5 100644
--- a/sys-utils/lscpu.c
+++ b/sys-utils/lscpu.c
@@ -690,7 +690,7 @@ is_vmware_platform(void)
 	if (sigsetjmp(segv_handler_env, 1))
 		return 0;
 
-	bzero(&act, sizeof(act));
+	memset(&act, 0, sizeof(act));
 	act.sa_sigaction = segv_handler;
 	act.sa_flags = SA_SIGINFO;
 
-- 
2.0.1

[PATCH 05/14] cfdisk: add braces to ensure operation order

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/cfdisk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disk-utils/cfdisk.c b/disk-utils/cfdisk.c
index b79579e..fb089f5 100644
--- a/disk-utils/cfdisk.c
+++ b/disk-utils/cfdisk.c
@@ -636,7 +636,7 @@ static void resize_on_signal(int dummy __attribute__((__unused__)))
 static void menu_refresh_size(struct cfdisk *cf)
 {
 	if (cf->menu && cf->menu->nitems)
-		cf->menu->page_sz = cf->menu->nitems / (LINES - 4) ? LINES - 4 : 0;
+		cf->menu->page_sz = (cf->menu->nitems / (LINES - 4)) ? LINES - 4 : 0;
 }
 
 static void menu_update_ignore(struct cfdisk *cf)
-- 
2.0.1

[PATCH 06/14] fdformat: match variable and print format types

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/fdformat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/disk-utils/fdformat.c b/disk-utils/fdformat.c
index b50ba30..e6ae8e4 100644
--- a/disk-utils/fdformat.c
+++ b/disk-utils/fdformat.c
@@ -35,7 +35,7 @@ static void format_disk(int ctrl)
 		if (ioctl(ctrl, FDFMTTRK, (long) &descr) < 0)
 			err(EXIT_FAILURE, "ioctl: FDFMTTRK");
 
-		printf("%3d\b\b\b", track);
+		printf("%3ud\b\b\b", track);
 		fflush(stdout);
 		if (param.head == 2) {
 			descr.head = 1;
@@ -63,7 +63,7 @@ static void verify_disk(char *name)
 	for (cyl = 0; cyl < param.track; cyl++) {
 		int read_bytes;
 
-		printf("%3d\b\b\b", cyl);
+		printf("%u3d\b\b\b", cyl);
 		fflush(stdout);
 		read_bytes = read(fd, data, cyl_size);
 		if (read_bytes != cyl_size) {
-- 
2.0.1

[PATCH 07/14] mkfs.cramfs: use defined failure name rather than magic value

[Sami Kerola]

>From manual page; 'On error, the value MAP_FAILED is returned, and errno
is set appropriately'.

Reference: http://man7.org/linux/man-pages/man2/mmap.2.html
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/mkfs.cramfs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/disk-utils/mkfs.cramfs.c b/disk-utils/mkfs.cramfs.c
index 6f412d8..bae68ec 100644
--- a/disk-utils/mkfs.cramfs.c
+++ b/disk-utils/mkfs.cramfs.c
@@ -167,11 +167,9 @@ do_mmap(char *path, unsigned int size, unsigned int mode){
 	}
 
 	start = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
-	if (-1 == (int) (long) start) {
-		close(fd);
-		err(MKFS_EX_ERROR, "mmap");
-	}
 	close(fd);
+	if (start == MAP_FAILED)
+		err(MKFS_EX_ERROR, "mmap");
 	return start;
 err:
 	free(start);
-- 
2.0.1

[PATCH 08/14] fdisk: avoid code duplication

[Sami Kerola]

To me having call to close() twice is less readable than one new
variable.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/fdisk.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/disk-utils/fdisk.c b/disk-utils/fdisk.c
index e9b4fec..d6bb737 100644
--- a/disk-utils/fdisk.c
+++ b/disk-utils/fdisk.c
@@ -725,16 +725,15 @@ static void print_all_devices_pt(struct fdisk_context *cxt)
 
 static sector_t get_dev_blocks(char *dev)
 {
-	int fd;
+	int fd, ret;
 	sector_t size;
 
 	if ((fd = open(dev, O_RDONLY)) < 0)
 		err(EXIT_FAILURE, _("cannot open %s"), dev);
-	if (blkdev_get_sectors(fd, &size) == -1) {
-		close(fd);
-		err(EXIT_FAILURE, _("BLKGETSIZE ioctl failed on %s"), dev);
-	}
+	ret = blkdev_get_sectors(fd, &size);
 	close(fd);
+	if (ret < 0)
+		err(EXIT_FAILURE, _("BLKGETSIZE ioctl failed on %s"), dev);
 	return size/2;
 }
 
-- 
2.0.1

[PATCH 09/14] dmesg: avoid unnecessary variable assignment

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/dmesg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sys-utils/dmesg.c b/sys-utils/dmesg.c
index 147a3ff..f95609f 100644
--- a/sys-utils/dmesg.c
+++ b/sys-utils/dmesg.c
@@ -612,7 +612,7 @@ static void safe_fwrite(const char *buf, size_t size, FILE *out)
 	for (i = 0; i < size; i++) {
 		const char *p = buf + i;
 		int rc, hex = 0;
-	        size_t len = 1;
+		size_t len;
 
 #ifdef HAVE_WIDECHAR
 		wchar_t wc;
@@ -629,6 +629,7 @@ static void safe_fwrite(const char *buf, size_t size, FILE *out)
 		}
 		i += len - 1;
 #else
+		len = 1;
 		if (!isprint((unsigned int) *p) &&
 			!isspace((unsigned int) *p))        /* non-printable */
 			hex = 1;
-- 
2.0.1

[PATCH 10/14] setpriv: avoid alloca() use xmalloc() instead

[Sami Kerola]

The getgroups() can return up to NGROUPS_MAX supplementary groups, that
is (since kernel 2.6.3) 65536 in total.  The git_t is 4 bytes, so maximum
request is 256 kilobytes.  When a system happen to have memory preasure
alloca() may not be able to allocate enough memory, making debugging
unnecessarily difficult.  IMHO 64 pages is significant enough amount of
memory to be properly error checked at a time of allocation.

Reference: http://www.gnu.org/software/libc/manual/html_node/Disadvantages-of-Alloca.html
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/setpriv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sys-utils/setpriv.c b/sys-utils/setpriv.c
index 65921be..ccfa993 100644
--- a/sys-utils/setpriv.c
+++ b/sys-utils/setpriv.c
@@ -254,9 +254,10 @@ static void dump_groups(void)
 		return;
 	}
 
-	groups = alloca(n * sizeof(gid_t));
+	groups = xmalloc(n * sizeof(gid_t));
 	n = getgroups(n, groups);
 	if (n < 0) {
+		free(groups);
 		warn("getgroups failed");
 		return;
 	}
@@ -273,6 +274,7 @@ static void dump_groups(void)
 		}
 	}
 	printf("\n");
+	free(groups);
 }
 
 static void dump(int dumplevel)
-- 
2.0.1

[PATCH 11/14] whereis: avoid printing uninitialized string

[Sami Kerola]

The patbuf[] needs to be filled before print out.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/whereis.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/misc-utils/whereis.c b/misc-utils/whereis.c
index ba83b15..5339aba 100644
--- a/misc-utils/whereis.c
+++ b/misc-utils/whereis.c
@@ -408,13 +408,12 @@ static void lookup(const char *pattern, struct wh_dirlist *ls, int want)
 	int count = 0;
 	char *wait = NULL, *p;
 
-	DBG(printf("lookup dirs for '%s' (%s)", patbuf, pattern));
-
 	/* canonicalize pattern -- remove path suffix etc. */
 	p = strrchr(pattern, '/');
 	p = p ? p + 1 : (char *) pattern;
 	strncpy(patbuf, p, PATH_MAX);
 	patbuf[PATH_MAX - 1] = '\0';
+	DBG(printf("lookup dirs for '%s' (%s)", patbuf, pattern));
 	p = strrchr(patbuf, '.');
 	if (p)
 		*p = '\0';
-- 
2.0.1

[PATCH 12/14] libfdisk: do not do the same thing twice in single if statement

[Sami Kerola]

The second argument was very likely meant to be 'be->offset'.
Maintainers review & sign-off will further verify this change being
correct.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 libfdisk/src/dos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libfdisk/src/dos.c b/libfdisk/src/dos.c
index 5453eb3..aff9755 100644
--- a/libfdisk/src/dos.c
+++ b/libfdisk/src/dos.c
@@ -1736,7 +1736,7 @@ static int cmp_ebr_offsets(const void *a, const void *b)
 	struct pte *ae = (struct pte *) a,
 		   *be = (struct pte *) b;
 
-	if (ae->offset == 0 && ae->offset == 0)
+	if (ae->offset == 0 && be->offset == 0)
 		return 0;
 	if (ae->offset == 0)
 		return 1;
-- 
2.0.1

[PATCH 13/14] libsmartcols: avoid variable dereference after release

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 libsmartcols/src/line.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsmartcols/src/line.c b/libsmartcols/src/line.c
index ab2f191..6e83fb4 100644
--- a/libsmartcols/src/line.c
+++ b/libsmartcols/src/line.c
@@ -199,9 +199,9 @@ int scols_line_remove_child(struct libscols_line *ln, struct libscols_line *chil
 	DBG(LINE, ul_debugobj(ln, "remove child %p", child));
 
 	list_del_init(&child->ln_children);
+	child->parent = NULL;
 	scols_unref_line(child);
 
-	child->parent = NULL;
 	scols_unref_line(ln);
 	return 0;
 }
-- 
2.0.1

[PATCH 14/14] lscpu: avoid double free

[Sami Kerola]

The double free was possible for architectures other than x86_64 or i386.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/lscpu-dmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys-utils/lscpu-dmi.c b/sys-utils/lscpu-dmi.c
index 251d4ac..6c870a1 100644
--- a/sys-utils/lscpu-dmi.c
+++ b/sys-utils/lscpu-dmi.c
@@ -258,7 +258,7 @@ int read_hypervisor_dmi(void)
 	if (rc)
 		goto done;
 	free(buf);
-
+	buf = NULL;
 memory_scan:
 #if defined(__x86_64__) || defined(__i386__)
 	/* Fallback to memory scan (x86, x86_64) */
-- 
2.0.1

Re: [PATCH 00/14] pull: some minor fixes

[Karel Zak]

On Sun, Jul 13, 2014 at 09:24:39PM +0100, Sami Kerola wrote:
>  disk-utils/cfdisk.c        | 2 +-
>  disk-utils/fdformat.c      | 4 ++--
>  disk-utils/fdisk.c         | 9 ++++-----
>  disk-utils/mkfs.cramfs.c   | 6 ++----
>  lib/strutils.c             | 2 +-
>  libfdisk/src/dos.c         | 2 +-
>  libfdisk/src/partition.c   | 2 +-
>  libmount/src/tab.c         | 2 +-
>  libsmartcols/src/line.c    | 2 +-
>  libsmartcols/src/version.c | 2 +-
>  login-utils/last.c         | 2 +-
>  misc-utils/cal.c           | 5 -----
>  misc-utils/whereis.c       | 3 +--
>  sys-utils/dmesg.c          | 3 ++-
>  sys-utils/fstrim.c         | 2 +-
>  sys-utils/lscpu-dmi.c      | 2 +-
>  sys-utils/lscpu.c          | 2 +-
>  sys-utils/setpriv.c        | 4 +++-
>  term-utils/setterm.c       | 3 +--
>  19 files changed, 26 insertions(+), 33 deletions(-)

 Applied, thanks!

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