[PATCH 0/8] pull: mostly mkswap and one swapoff chanage

[PATCH 0/8] pull: mostly mkswap and one swapoff chanage

[Sami Kerola]

Hello,

This pull request contains corrected version of swapoff(8) change, that was
first sent at 31 October 2014 23:57 and had issues.

Rest of the changes are to mkswap, that needed a bit dusting.  Pretty old
sparc 32bit support for kernels about 15 years ago is removed.  The command
got all familiar util-linux convention like 'struct mkswap_control', that
made easier to see what exactly is the data functions are handling.  The
rest of the changes are straight forward clarifications.

----------------------------------------------------------------
The following changes since commit 54f2e4563d6c7064df9f2a201ffe928709c29333:
  swapon: remove extra word from usage (2014-10-31 10:58:27 +0100)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git 2014wk43
for you to fetch changes up to 9e55f54b2f1a740da508c5f192371f5df900680f:
  mkswap: various minor improvement (2014-11-02 19:50:00 +0000)
----------------------------------------------------------------
Sami Kerola (8):
  swapoff: make LABEL= and UUID= work when turning off swap files
  mkswap: remove system architecture specific max swap size checks
  mkswap: remove unnecessary size check
  mkswap: use err() rather than perror() && exit()
  mkswap: add struct mkswap_control to remove global variables
  mkswap: make remaining functions to take control structure as argument
  mkswap: set variable only when it's value is known
  mkswap: various minor improvement

 disk-utils/mkswap.c     | 435 ++++++++++++++++--------------------------------
 include/swapheader.h    |   2 -
 sys-utils/Makemodule.am |  13 +-
 sys-utils/swapoff.c     |  34 ++--
 4 files changed, 177 insertions(+), 307 deletions(-)

-- 
2.1.3

[PATCH 1/8] swapoff: make LABEL= and UUID= work when turning off swap files

[Sami Kerola]

Before this commit the swapoff was unable to find swap files by UUID or
LABEL.  For example if one had:

$ swapon --show=name,uuid,label
NAME      UUID                                 LABEL
/swapfile bf6237f4-db89-4761-a1a3-275cfc68b114 xyzzy

then turning off the swap had issues:

$ swapoff -U bf6237f4-db89-4761-a1a3-275cfc68b114
swapoff: cannot find the device for bf6237f4-db89-4761-a1a3-275cfc68b114
$ swapoff -L xyzzy
swapoff: cannot find the device for xyzzy

This version of the swapoff is also capable to turn off multiple swap
devices that share the same UUID or LABEL.  Earlier only the first found
match was turned off.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/Makemodule.am | 13 ++++++++++---
 sys-utils/swapoff.c     | 34 +++++++++++++++++++++++-----------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index f540d38..b256fda 100644
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -279,9 +279,16 @@ swapon_LDADD = $(LDADD) \
 swapoff_SOURCES = \
 	sys-utils/swapoff.c \
 	sys-utils/swapon-common.c \
-	sys-utils/swapon-common.h
-swapoff_CFLAGS = $(AM_CFLAGS) -I$(ul_libmount_incdir)
-swapoff_LDADD = $(LDADD) libmount.la
+	sys-utils/swapon-common.h \
+	lib/swapprober.c \
+	include/swapprober.h
+swapoff_CFLAGS = $(AM_CFLAGS) \
+	-I$(ul_libblkid_incdir) \
+	-I$(ul_libmount_incdir)
+swapoff_LDADD = $(LDADD) \
+	libblkid.la \
+	libcommon.la \
+	libmount.la
 endif
 
 if BUILD_LSCPU
diff --git a/sys-utils/swapoff.c b/sys-utils/swapoff.c
index 182ce95..20b9249 100644
--- a/sys-utils/swapoff.c
+++ b/sys-utils/swapoff.c
@@ -9,6 +9,7 @@
 #include "nls.h"
 #include "c.h"
 #include "closestream.h"
+#include "swapprober.h"
 
 #include "swapon-common.h"
 
@@ -49,16 +50,27 @@ static int do_swapoff(const char *orig_special, int quiet, int canonic)
 	return -1;
 }
 
-static int swapoff_by_label(const char *label, int quiet)
+static int swapoff_by(const char *type, const char *request, int quiet)
 {
-	const char *special = mnt_resolve_tag("LABEL", label, mntcache);
-	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(label);
-}
+	struct libmnt_table *tb = get_swaps();
+	struct libmnt_iter *itr = mnt_new_iter(MNT_ITER_BACKWARD);
+	struct libmnt_fs *fs;
+	int ret = 0, notfound = -1;
 
-static int swapoff_by_uuid(const char *uuid, int quiet)
-{
-	const char *special = mnt_resolve_tag("UUID", uuid, mntcache);
-	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(uuid);
+	while (tb && mnt_table_find_next_fs(tb, itr, match_swap, NULL, &fs) == 0) {
+		const char *dev = mnt_fs_get_source(fs);
+		blkid_probe pr;
+		const char *data;
+
+		pr = get_swap_prober(dev);
+		blkid_probe_lookup_value(pr, type, &data, NULL);
+		if (data && !strcmp(request, data)) {
+			notfound = 0;
+			ret |= do_swapoff(dev, quiet, CANONIC);
+		}
+	}
+	ret += notfound;
+	return !ret ? 0 : cannot_find(request);
 }
 
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
@@ -118,7 +130,7 @@ static int swapoff_all(void)
 
 	while (tb && mnt_table_find_next_fs(tb, itr, match_swap, NULL, &fs) == 0) {
 		if (!is_active_swap(mnt_fs_get_source(fs)))
-			do_swapoff(mnt_fs_get_source(fs), QUIET, !CANONIC);
+			do_swapoff(mnt_fs_get_source(fs), QUIET, CANONIC);
 	}
 
 	mnt_free_iter(itr);
@@ -178,10 +190,10 @@ int main(int argc, char *argv[])
 	mntcache = mnt_new_cache();
 
 	for (i = 0; i < numof_labels(); i++)
-		status |= swapoff_by_label(get_label(i), !QUIET);
+		status |= swapoff_by("LABEL", get_label(i), !QUIET);
 
 	for (i = 0; i < numof_uuids(); i++)
-		status |= swapoff_by_uuid(get_uuid(i), !QUIET);
+		status |= swapoff_by("UUID", get_uuid(i), !QUIET);
 
 	while (*argv != NULL)
 		status |= do_swapoff(*argv++, !QUIET, !CANONIC);
-- 
2.1.3

Re: [PATCH 1/8] swapoff: make LABEL= and UUID= work when turning off swap files

[Karel Zak]

On Sun, Nov 02, 2014 at 08:26:24PM +0000, Sami Kerola wrote:
> -static int swapoff_by_label(const char *label, int quiet)
> +static int swapoff_by(const char *type, const char *request, int quiet)
>  {
> -	const char *special = mnt_resolve_tag("LABEL", label, mntcache);
> -	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(label);
> -}
> +	struct libmnt_table *tb = get_swaps();
> +	struct libmnt_iter *itr = mnt_new_iter(MNT_ITER_BACKWARD);
> +	struct libmnt_fs *fs;
> +	int ret = 0, notfound = -1;
>  
> -static int swapoff_by_uuid(const char *uuid, int quiet)
> -{
> -	const char *special = mnt_resolve_tag("UUID", uuid, mntcache);
> -	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(uuid);
> +	while (tb && mnt_table_find_next_fs(tb, itr, match_swap, NULL, &fs) == 0) {
> +		const char *dev = mnt_fs_get_source(fs);
> +		blkid_probe pr;
> +		const char *data;
> +
> +		pr = get_swap_prober(dev);
> +		blkid_probe_lookup_value(pr, type, &data, NULL);
> +		if (data && !strcmp(request, data)) {
> +			notfound = 0;
> +			ret |= do_swapoff(dev, quiet, CANONIC);
> +		}
> +	}
> +	ret += notfound;
> +	return !ret ? 0 : cannot_find(request);
>  }

You have forced all swapoff to use /proc/swaps to translate
UUID/LABEL to device or file name. It's bad idea because

  * it's expensive, mnt_resolve_* function uses udev symlinks, one
    readlink() is faster than libblkid machinery

  * dependence on /proc/swaps should be optional, the /proc filesystem
    does not have to be mounted

IMHO it would be better to use libblkid and /proc/swaps as fallback
solution only.


>  static void __attribute__ ((__noreturn__)) usage(FILE * out)
> @@ -118,7 +130,7 @@ static int swapoff_all(void)
>  
>  	while (tb && mnt_table_find_next_fs(tb, itr, match_swap, NULL, &fs) == 0) {
>  		if (!is_active_swap(mnt_fs_get_source(fs)))
> -			do_swapoff(mnt_fs_get_source(fs), QUIET, !CANONIC);
> +			do_swapoff(mnt_fs_get_source(fs), QUIET, CANONIC);
>  	}

 this seems like typo (or do you expect that all fstab stuff is
 canonical?)

>  	for (i = 0; i < numof_labels(); i++)
> -		status |= swapoff_by_label(get_label(i), !QUIET);
> +		status |= swapoff_by("LABEL", get_label(i), !QUIET);
>  
>  	for (i = 0; i < numof_uuids(); i++)
> -		status |= swapoff_by_uuid(get_uuid(i), !QUIET);
> +		status |= swapoff_by("UUID", get_uuid(i), !QUIET);
  
 I like the idea to have only one swapoff_by() function.

>  	while (*argv != NULL)
>  		status |= do_swapoff(*argv++, !QUIET, !CANONIC);

 BTW, you have no changed tags resolving in do_swapoff(), so I guess

    swaponff UUID=<uuid> 

does not work with your patch, the patch fixes only -U <uuid> use
case.

I have applied a little different patch to fix the problem.

    Karel


>From 52f2fd9bcc9bbc2a0234114f11870d943de76652 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@redhat.com>
Date: Fri, 7 Nov 2014 12:08:11 +0100
Subject: [PATCH] swapoff: swapoff swap files by LABEL and UUID

 # swapon --show=NAME,UUID
 NAME                   UUID
 /dev/sda3              8d52fca3-bf48-41d6-b826-2315e518a305
 /home/fs-images/2g.img 6fa72b96-b802-441f-a31c-091d65c0212c

 # swapoff UUID=6fa72b96-b802-441f-a31c-091d65c0212c
 swapoff: cannot find the device for UUID=6fa72b96-b802-441f-a31c-091d65c0212c

Reported-by: Sami Kerola <kerolasa@iki.fi>
Signed-off-by: Karel Zak <kzak@redhat.com>
---
 sys-utils/Makemodule.am | 13 +++++++--
 sys-utils/swapoff.c     | 74 +++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index f540d38..12fa632 100644
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -279,9 +279,16 @@ swapon_LDADD = $(LDADD) \
 swapoff_SOURCES = \
 	sys-utils/swapoff.c \
 	sys-utils/swapon-common.c \
-	sys-utils/swapon-common.h
-swapoff_CFLAGS = $(AM_CFLAGS) -I$(ul_libmount_incdir)
-swapoff_LDADD = $(LDADD) libmount.la
+	sys-utils/swapon-common.h \
+	lib/swapprober.c \
+	include/swapprober.h
+swapoff_CFLAGS = $(AM_CFLAGS) \
+	-I$(ul_libblkid_incdir) \
+	-I$(ul_libmount_incdir)
+swapoff_LDADD = $(LDADD) \
+	libmount.la \
+	libblkid.la \
+	libcommon.la
 endif
 
 if BUILD_LSCPU
diff --git a/sys-utils/swapoff.c b/sys-utils/swapoff.c
index 182ce95..b725462 100644
--- a/sys-utils/swapoff.c
+++ b/sys-utils/swapoff.c
@@ -8,8 +8,10 @@
 
 #include "nls.h"
 #include "c.h"
+#include "xalloc.h"
 #include "closestream.h"
 
+#include "swapprober.h"
 #include "swapon-common.h"
 
 #ifndef SWAPON_HAS_TWO_ARGS
@@ -24,6 +26,58 @@ static int all;
 #define QUIET	1
 #define CANONIC	1
 
+/*
+ * This function works like mnt_resolve_tag(), but it's able to read UUiD/LABEL
+ * from regular swap files too (according to entries in /proc/swaps). Note that
+ * mnt_resolve_tag() and mnt_resolve_spec() works with system visible block
+ * devices only.
+ */
+static char *swapoff_resolve_tag(const char *name, const char *value,
+				 struct libmnt_cache *cache)
+{
+	char *path;
+	struct libmnt_table *tb;
+	struct libmnt_iter *itr;
+	struct libmnt_fs *fs;
+
+	/* this is usual case for block devices (and it's really fast as it uses
+	 * udev /dev/disk/by-* symlinks by default */
+	path = mnt_resolve_tag(name, value, cache);
+	if (path)
+		return path;
+
+	/* try regular files from /proc/swaps */
+	tb = get_swaps();
+	if (!tb)
+		return NULL;
+
+	itr = mnt_new_iter(MNT_ITER_BACKWARD);
+	if (!itr)
+		err(EXIT_FAILURE, _("failed to initialize libmount iterator"));
+
+	while (tb && mnt_table_next_fs(tb, itr, &fs) == 0) {
+		blkid_probe pr = NULL;
+		const char *src = mnt_fs_get_source(fs);
+		const char *type = mnt_fs_get_swaptype(fs);
+		const char *data = NULL;
+
+		if (!src || !type || strcmp(type, "file") != 0)
+			continue;
+		pr = get_swap_prober(src);
+		if (!pr)
+			continue;
+		blkid_probe_lookup_value(pr, name, &data, NULL);
+		if (data && strcmp(data, value) == 0)
+			path = xstrdup(src);
+		blkid_free_probe(pr);
+		if (path)
+			break;
+	}
+
+	mnt_free_iter(itr);
+	return path;
+}
+
 static int do_swapoff(const char *orig_special, int quiet, int canonic)
 {
         const char *special = orig_special;
@@ -32,7 +86,11 @@ static int do_swapoff(const char *orig_special, int quiet, int canonic)
 		printf(_("swapoff %s\n"), orig_special);
 
 	if (!canonic) {
+		char *n, *v;
+
 		special = mnt_resolve_spec(orig_special, mntcache);
+		if (!special && blkid_parse_tag_string(orig_special, &n, &v) == 0)
+			special = swapoff_resolve_tag(n, v, mntcache);
 		if (!special)
 			return cannot_find(orig_special);
 	}
@@ -49,16 +107,10 @@ static int do_swapoff(const char *orig_special, int quiet, int canonic)
 	return -1;
 }
 
-static int swapoff_by_label(const char *label, int quiet)
-{
-	const char *special = mnt_resolve_tag("LABEL", label, mntcache);
-	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(label);
-}
-
-static int swapoff_by_uuid(const char *uuid, int quiet)
+static int swapoff_by(const char *name, const char *value, int quiet)
 {
-	const char *special = mnt_resolve_tag("UUID", uuid, mntcache);
-	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(uuid);
+	const char *special = swapoff_resolve_tag(name, value, mntcache);
+	return special ? do_swapoff(special, quiet, CANONIC) : cannot_find(value);
 }
 
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
@@ -178,10 +230,10 @@ int main(int argc, char *argv[])
 	mntcache = mnt_new_cache();
 
 	for (i = 0; i < numof_labels(); i++)
-		status |= swapoff_by_label(get_label(i), !QUIET);
+		status |= swapoff_by("LABEL", get_label(i), !QUIET);
 
 	for (i = 0; i < numof_uuids(); i++)
-		status |= swapoff_by_uuid(get_uuid(i), !QUIET);
+		status |= swapoff_by("UUID", get_uuid(i), !QUIET);
 
 	while (*argv != NULL)
 		status |= do_swapoff(*argv++, !QUIET, !CANONIC);
-- 
1.9.3

[PATCH 2/8] mkswap: remove system architecture specific max swap size checks

[Sami Kerola]

Since kernel version 2.3.4 (June 1999) all architectures has used
uint32_t as maximum number or pages in a swap device or file, there is no
longer need to support systems earlier than that.

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

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index 5eb4e6a..a1b864d 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -75,75 +75,6 @@ static int check = 0;
 
 #define SELINUX_SWAPFILE_TYPE	"swapfile_t"
 
-#ifdef __sparc__
-# ifdef __arch64__
-#  define is_sparc64() 1
-#  define is_be64() 1
-# else /* sparc32 */
-static int
-is_sparc64(void)
-{
-	struct utsname un;
-	static int sparc64 = -1;
-
-	if (sparc64 != -1)
-		return sparc64;
-	sparc64 = 0;
-
-	if (uname(&un) < 0)
-		return 0;
-	if (! strcmp(un.machine, "sparc64")) {
-		sparc64 = 1;
-		return 1;
-	}
-	if (strcmp(un.machine, "sparc"))
-		return 0; /* Should not happen */
-
-#ifdef HAVE_PERSONALITY
-	{
-		extern int personality(unsigned long);
-		int oldpers;
-#define PERS_LINUX          0x00000000
-#define PERS_LINUX_32BIT    0x00800000
-#define PERS_LINUX32        0x00000008
-
-		oldpers = personality(PERS_LINUX_32BIT);
-		if (oldpers != -1) {
-			if (personality(PERS_LINUX) != -1) {
-				uname(&un);
-				if (! strcmp(un.machine, "sparc64")) {
-					sparc64 = 1;
-					oldpers = PERS_LINUX32;
-				}
-			}
-			personality(oldpers);
-		}
-	}
-#endif
-
-	return sparc64;
-}
-#  define is_be64() is_sparc64()
-# endif /* sparc32 */
-#else /* !sparc */
-# define is_be64() 0
-#endif
-
-/*
- * The definition of the union swap_header uses the kernel constant PAGE_SIZE.
- * Unfortunately, on some architectures this depends on the hardware model, and
- * can only be found at run time -- we use getpagesize(), so that we do not
- * need separate binaries e.g. for sun4, sun4c/d/m and sun4u.
- *
- * Even more unfortunately, getpagesize() does not always return the right
- * information. For example, libc4, libc5 and glibc 2.0 do not use the system
- * call but invent a value themselves (EXEC_PAGESIZE or NBPG * CLSIZE or NBPC),
- * and thus it may happen that e.g. on a sparc kernel PAGE_SIZE=4096 and
- * getpagesize() returns 8192.
- *
- * What to do? Let us allow the user to specify the pagesize explicitly.
- *
- */
 static unsigned int user_pagesize;
 static unsigned int pagesize;
 static unsigned long *signature_page = NULL;
@@ -214,61 +145,6 @@ write_uuid_and_label(unsigned char *uuid, char *volume_name)
 	}
 }
 
-/*
- * Find out what the maximum amount of swap space is that the kernel will
- * handle.  This wouldn't matter if the kernel just used as much of the
- * swap space as it can handle, but until 2.3.4 it would return an error
- * to swapon() if the swapspace was too large.
- */
-/* Before 2.2.0pre9 */
-#define V1_OLD_MAX_PAGES	((0x7fffffff / pagesize) - 1)
-/* Since 2.2.0pre9, before 2.3.4:
-   error if nr of pages >= SWP_OFFSET(SWP_ENTRY(0,~0UL))
-   with variations on
-	#define SWP_ENTRY(type,offset) (((type) << 1) | ((offset) << 8))
-	#define SWP_OFFSET(entry) ((entry) >> 8)
-   on the various architectures. Below the result - yuk.
-
-   Machine	pagesize	SWP_ENTRY	SWP_OFFSET	bound+1	oldbound+2
-   i386		2^12		o<<8		e>>8		1<<24	1<<19
-   mips		2^12		o<<15		e>>15		1<<17	1<<19
-   alpha	2^13		o<<40		e>>40		1<<24	1<<18
-   m68k		2^12		o<<12		e>>12		1<<20	1<<19
-   sparc	2^{12,13}	(o&0x3ffff)<<9	(e>>9)&0x3ffff	1<<18	1<<{19,18}
-   sparc64	2^13		o<<13		e>>13		1<<51	1<<18
-   ppc		2^12		o<<8		e>>8		1<<24	1<<19
-   armo		2^{13,14,15}	o<<8		e>>8		1<<24	1<<{18,17,16}
-   armv		2^12		o<<9		e>>9		1<<23	1<<19
-
-   assuming that longs have 64 bits on alpha and sparc64 and 32 bits elsewhere.
-
-   The bad part is that we need to know this since the kernel will
-   refuse a swap space if it is too large.
-*/
-/* patch from jj - why does this differ from the above? */
-/* 32bit kernels have a second limitation of 2GB, sparc64 is limited by
-   the size of virtual address space allocation for vmalloc */
-#if defined(__alpha__)
-#define V1_MAX_PAGES           ((1 << 24) - 1)
-#elif defined(__mips__)
-#define V1_MAX_PAGES           ((1 << 17) - 1)
-#elif defined(__sparc__)
-#define V1_MAX_PAGES           (is_sparc64() ? ((3 << 29) - 1) : ((1 << 18) - 1))
-#elif defined(__ia64__)
-/*
- * The actual size will depend on the amount of virtual address space
- * available to vmalloc the swap map.
- */
-#define V1_MAX_PAGES          ((1UL << 54) - 1)
-#else
-#define V1_MAX_PAGES           V1_OLD_MAX_PAGES
-#endif
-/* man page now says:
-The maximum useful size of a swap area now depends on the architecture.
-It is roughly 2GB on i386, PPC, m68k, ARM, 1GB on sparc, 512MB on mips,
-128GB on alpha and 3TB on sparc64.
-*/
-
 #define MAX_BADPAGES	((pagesize-1024-128*sizeof(int)-10)/sizeof(int))
 #define MIN_GOODPAGES	10
 
@@ -442,7 +318,6 @@ main(int argc, char **argv) {
 	struct stat statbuf;
 	struct swap_header_v1_2 *hdr;
 	int c;
-	unsigned long long maxpages;
 	unsigned long long goodpages;
 	unsigned long long sz;
 	off_t offset;
@@ -553,18 +428,9 @@ main(int argc, char **argv) {
 		errx(EXIT_FAILURE,
 		     _("error: swap area needs to be at least %ld KiB"),
 		     (long)(MIN_GOODPAGES * pagesize / 1024));
-
-#ifdef __linux__
-	if (get_linux_version() >= KERNEL_VERSION(2,3,4))
-		maxpages = UINT_MAX + 1ULL;
-	else if (get_linux_version() >= KERNEL_VERSION(2,2,1))
-		maxpages = V1_MAX_PAGES;
-	else
-#endif
-		maxpages = V1_OLD_MAX_PAGES;
-
-	if (PAGES > maxpages) {
-		PAGES = maxpages;
+	if (PAGES > UINT32_MAX) {
+		/* true when swap is bigger than 17.59 terabytes */
+		PAGES = UINT32_MAX;
 		warnx(_("warning: truncating swap area to %llu KiB"),
 			PAGES * pagesize / 1024);
 	}
-- 
2.1.3

[PATCH 3/8] mkswap: remove unnecessary size check

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/mkswap.c  | 6 ------
 include/swapheader.h | 2 --
 2 files changed, 8 deletions(-)

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index a1b864d..b2713ab 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -115,12 +115,6 @@ write_uuid_and_label(unsigned char *uuid, char *volume_name)
 {
 	struct swap_header_v1_2 *h;
 
-	/* Sanity check */
-	if (sizeof(struct swap_header_v1_2) != SWAP_HEADER_SIZE) {
-		warnx(_("Bad swap header size, no label written."));
-		return;
-	}
-
 	h = (struct swap_header_v1_2 *) signature_page;
 	if (uuid)
 		memcpy(h->uuid, uuid, sizeof(h->uuid));
diff --git a/include/swapheader.h b/include/swapheader.h
index 1d91e41..3fce0d0 100644
--- a/include/swapheader.h
+++ b/include/swapheader.h
@@ -20,6 +20,4 @@ struct swap_header_v1_2 {
 	uint32_t      badpages[1];
 };
 
-#define SWAP_HEADER_SIZE (sizeof(struct swap_header_v1_2))
-
 #endif /* _SWAPHEADER_H */
-- 
2.1.3

[PATCH 4/8] mkswap: use err() rather than perror() && exit()

[Sami Kerola]

The messsages in err() are verified from po/util-linux.pot to be already
part of translations.

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

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index b2713ab..14adf0f 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -209,10 +209,8 @@ get_size(const char *file)
 	unsigned long long size;
 
 	fd = open(file, O_RDONLY);
-	if (fd < 0) {
-		perror(file);
-		exit(EXIT_FAILURE);
-	}
+	if (fd < 0)
+		err(EXIT_FAILURE, _("cannot open %s"), file);
 	if (blkdev_get_size(fd, &size) == 0)
 		size /= pagesize;
 
@@ -434,19 +432,14 @@ main(int argc, char **argv) {
 			"%s is mounted; will not make swapspace"),
 			device_name);
 
-	if (stat(device_name, &statbuf) < 0) {
-		perror(device_name);
-		exit(EXIT_FAILURE);
-	}
+	if (stat(device_name, &statbuf) < 0)
+		err(EXIT_FAILURE, _("stat failed %s"), device_name);
 	if (S_ISBLK(statbuf.st_mode))
 		DEV = open(device_name, O_RDWR | O_EXCL);
 	else
 		DEV = open(device_name, O_RDWR);
-
-	if (DEV < 0) {
-		perror(device_name);
-		exit(EXIT_FAILURE);
-	}
+	if (DEV < 0)
+		err(EXIT_FAILURE, _("cannot open %s"), device_name);
 
 	if (!S_ISBLK(statbuf.st_mode))
 		check=0;
-- 
2.1.3

[PATCH 5/8] mkswap: add struct mkswap_control to remove global variables

[Sami Kerola]

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

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index 14adf0f..7f41428 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -67,71 +67,79 @@
 # include <blkid.h>
 #endif
 
-static char *device_name = NULL;
-static int DEV = -1;
-static unsigned long long PAGES = 0;
-static unsigned long badpages = 0;
-static int check = 0;
+#define MIN_GOODPAGES	10
 
 #define SELINUX_SWAPFILE_TYPE	"swapfile_t"
 
-static unsigned int user_pagesize;
-static unsigned int pagesize;
-static unsigned long *signature_page = NULL;
+struct mkswap_control {
+	struct swap_header_v1_2 *hdr;
+	char *device_name;
+	int fd;
+	unsigned long long nr_pages;
+	unsigned long badpages;
+	int user_pagesize;
+	int pagesize;
+	void *signature_page;
+	char *opt_label;
+	unsigned char *uuid;
+	unsigned int
+		check:1,
+		force:1;
+};
 
 static void
-init_signature_page(void)
+init_signature_page(struct mkswap_control *ctl)
 {
 
-	unsigned int kernel_pagesize = pagesize = getpagesize();
+	int kernel_pagesize = ctl->pagesize = getpagesize();
 
-	if (user_pagesize) {
-		if (!is_power_of_2(user_pagesize) ||
-		    user_pagesize < sizeof(struct swap_header_v1_2) + 10)
+	if (ctl->user_pagesize) {
+		if (ctl->user_pagesize < 0 || !is_power_of_2(ctl->user_pagesize) ||
+		    (size_t) ctl->user_pagesize < sizeof(struct swap_header_v1_2) + 10)
 			errx(EXIT_FAILURE,
 				_("Bad user-specified page size %u"),
-				user_pagesize);
-		pagesize = user_pagesize;
+				ctl->user_pagesize);
+		ctl->pagesize = ctl->user_pagesize;
 	}
 
-	if (user_pagesize && user_pagesize != kernel_pagesize)
+	if (ctl->user_pagesize && ctl->user_pagesize != kernel_pagesize)
 		warnx(_("Using user-specified page size %d, "
 				"instead of the system value %d"),
-				pagesize, kernel_pagesize);
+				ctl->pagesize, kernel_pagesize);
 
-	signature_page = (unsigned long *) xcalloc(1, pagesize);
+	ctl->signature_page = (unsigned long *) xcalloc(1, ctl->pagesize);
 }
 
 static void
-write_signature(char *sig)
+write_signature(const struct mkswap_control *ctl)
 {
-	char *sp = (char *) signature_page;
+	char *sp = (char *) ctl->signature_page;
 
-	strncpy(sp + pagesize - SWAP_SIGNATURE_SZ, sig, SWAP_SIGNATURE_SZ);
+	strncpy(sp + ctl->pagesize - SWAP_SIGNATURE_SZ, SWAP_SIGNATURE, SWAP_SIGNATURE_SZ);
 }
 
 static void
-write_uuid_and_label(unsigned char *uuid, char *volume_name)
+write_uuid_and_label(const struct mkswap_control *ctl)
 {
 	struct swap_header_v1_2 *h;
 
-	h = (struct swap_header_v1_2 *) signature_page;
-	if (uuid)
-		memcpy(h->uuid, uuid, sizeof(h->uuid));
-	if (volume_name) {
-		xstrncpy(h->volume_name, volume_name, sizeof(h->volume_name));
-		if (strlen(volume_name) > strlen(h->volume_name))
+	h = (struct swap_header_v1_2 *) ctl->signature_page;
+	if (ctl->uuid)
+		memcpy(h->uuid, ctl->uuid, sizeof(h->uuid));
+	if (ctl->opt_label) {
+		xstrncpy(h->volume_name, ctl->opt_label, sizeof(h->volume_name));
+		if (strlen(ctl->opt_label) > strlen(h->volume_name))
 			warnx(_("Label was truncated."));
 	}
-	if (uuid || volume_name) {
-		if (volume_name)
+	if (ctl->uuid || ctl->opt_label) {
+		if (ctl->opt_label)
 			printf("LABEL=%s, ", h->volume_name);
 		else
 			printf(_("no label, "));
 #ifdef HAVE_LIBUUID
-		if (uuid) {
+		if (ctl->uuid) {
 			char uuid_string[37];
-			uuid_unparse(uuid, uuid_string);
+			uuid_unparse(ctl->uuid, uuid_string);
 			printf("UUID=%s\n", uuid_string);
 		} else
 #endif
@@ -139,9 +147,6 @@ write_uuid_and_label(unsigned char *uuid, char *volume_name)
 	}
 }
 
-#define MAX_BADPAGES	((pagesize-1024-128*sizeof(int)-10)/sizeof(int))
-#define MIN_GOODPAGES	10
-
 static void __attribute__ ((__noreturn__)) usage(FILE *out)
 {
 	fprintf(out,
@@ -164,55 +169,55 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
 }
 
 static void
-page_bad(int page)
+page_bad(struct mkswap_control *ctl, int page)
 {
-	struct swap_header_v1_2 *p = (struct swap_header_v1_2 *) signature_page;
+	struct swap_header_v1_2 *p = (struct swap_header_v1_2 *)ctl->signature_page;
+	const unsigned long max_badpages = (ctl->pagesize - 1024 - 128 * sizeof(int) - 10) / sizeof(int);
 
-	if (badpages == MAX_BADPAGES)
+	if (ctl->badpages == max_badpages)
 		errx(EXIT_FAILURE, _("too many bad pages"));
-	p->badpages[badpages] = page;
-	badpages++;
+	p->badpages[ctl->badpages] = page;
+	ctl->badpages++;
 }
 
 static void
-check_blocks(void)
+check_blocks(struct mkswap_control *ctl)
 {
 	unsigned int current_page;
 	int do_seek = 1;
 	char *buffer;
 
-	buffer = xmalloc(pagesize);
+	buffer = xmalloc(ctl->pagesize);
 	current_page = 0;
-	while (current_page < PAGES) {
-
+	while (current_page < ctl->nr_pages) {
 		ssize_t rc;
 
-		if (do_seek && lseek(DEV,current_page*pagesize,SEEK_SET) !=
-		    current_page*pagesize)
+		if (do_seek && lseek(ctl->fd, current_page * ctl->pagesize, SEEK_SET) !=
+		    current_page * ctl->pagesize)
 			errx(EXIT_FAILURE, _("seek failed in check_blocks"));
 
-		rc = read(DEV, buffer, pagesize);
-		do_seek = (rc < 0 || (size_t) rc != pagesize);
+		rc = read(ctl->fd, buffer, ctl->pagesize);
+		do_seek = (rc < 0 || rc != ctl->pagesize);
 		if (do_seek)
-			page_bad(current_page);
+			page_bad(ctl, current_page);
 		current_page++;
 	}
-	printf(P_("%lu bad page\n", "%lu bad pages\n", badpages), badpages);
+	printf(P_("%lu bad page\n", "%lu bad pages\n", ctl->badpages), ctl->badpages);
 	free(buffer);
 }
 
 /* return size in pages */
 static unsigned long long
-get_size(const char *file)
+get_size(const struct mkswap_control *ctl)
 {
 	int fd;
 	unsigned long long size;
 
-	fd = open(file, O_RDONLY);
+	fd = open(ctl->device_name, O_RDONLY);
 	if (fd < 0)
-		err(EXIT_FAILURE, _("cannot open %s"), file);
+		err(EXIT_FAILURE, _("cannot open %s"), ctl->device_name);
 	if (blkdev_get_size(fd, &size) == 0)
-		size /= pagesize;
+		size /= ctl->pagesize;
 
 	close(fd);
 	return size;
@@ -307,17 +312,14 @@ wipe_device(int fd, const char *devname, int force)
 
 int
 main(int argc, char **argv) {
+	struct mkswap_control ctl = { .fd = -1, 0 };
 	struct stat statbuf;
-	struct swap_header_v1_2 *hdr;
 	int c;
 	unsigned long long goodpages;
 	unsigned long long sz;
 	off_t offset;
-	int force = 0;
 	int version = SWAP_VERSION;
 	char *block_count = 0;
-	char *opt_label = NULL;
-	unsigned char *uuid = NULL;
 #ifdef HAVE_LIBUUID
 	const char *opt_uuid = NULL;
 	uuid_t uuid_dat;
@@ -342,19 +344,22 @@ main(int argc, char **argv) {
 	while((c = getopt_long(argc, argv, "cfp:L:v:U:Vh", longopts, NULL)) != -1) {
 		switch (c) {
 		case 'c':
-			check=1;
+			ctl.check = 1;
 			break;
 		case 'f':
-			force=1;
+			ctl.force = 1;
 			break;
 		case 'p':
-			user_pagesize = strtou32_or_err(optarg, _("parsing page size failed"));
+			ctl.user_pagesize = strtou32_or_err(optarg, _("parsing page size failed"));
 			break;
 		case 'L':
-			opt_label = optarg;
+			ctl.opt_label = optarg;
 			break;
 		case 'v':
 			version = strtos32_or_err(optarg, _("parsing version number failed"));
+			if (version != SWAP_VERSION)
+				errx(EXIT_FAILURE,
+					_("swapspace version %d is not supported"), version);
 			break;
 		case 'U':
 #ifdef HAVE_LIBUUID
@@ -374,7 +379,7 @@ main(int argc, char **argv) {
 		}
 	}
 	if (optind < argc)
-		device_name = argv[optind++];
+		ctl.device_name = argv[optind++];
 	if (optind < argc)
 		block_count = argv[optind++];
 	if (optind != argc) {
@@ -382,22 +387,18 @@ main(int argc, char **argv) {
 		usage(stderr);
 	}
 
-	if (version != SWAP_VERSION)
-		errx(EXIT_FAILURE,
-			_("swapspace version %d is not supported"), version);
-
 #ifdef HAVE_LIBUUID
 	if(opt_uuid) {
 		if (uuid_parse(opt_uuid, uuid_dat) != 0)
 			errx(EXIT_FAILURE, _("error: parsing UUID failed"));
 	} else
 		uuid_generate(uuid_dat);
-	uuid = uuid_dat;
+	ctl.uuid = uuid_dat;
 #endif
 
-	init_signature_page();	/* get pagesize */
+	init_signature_page(&ctl);	/* get pagesize and allocate signature page */
 
-	if (!device_name) {
+	if (!ctl.device_name) {
 		warnx(_("error: Nowhere to set up swap on?"));
 		usage(stderr);
 	}
@@ -405,75 +406,75 @@ main(int argc, char **argv) {
 		/* this silly user specified the number of blocks explicitly */
 		uint64_t blks = strtou64_or_err(block_count,
 					_("invalid block count argument"));
-		PAGES = blks / (pagesize / 1024);
+		ctl.nr_pages = blks / (ctl.pagesize / 1024);
 	}
-	sz = get_size(device_name);
-	if (!PAGES)
-		PAGES = sz;
-	else if (PAGES > sz && !force)
+	sz = get_size(&ctl);
+	if (!ctl.nr_pages)
+		ctl.nr_pages = sz;
+	else if (ctl.nr_pages > sz && !ctl.force)
 		errx(EXIT_FAILURE,
 			_("error: "
 			  "size %llu KiB is larger than device size %llu KiB"),
-			PAGES*(pagesize/1024), sz*(pagesize/1024));
+			ctl.nr_pages * (ctl.pagesize / 1024), sz * (ctl.pagesize / 1024));
 
-	if (PAGES < MIN_GOODPAGES)
+	if (ctl.nr_pages < MIN_GOODPAGES)
 		errx(EXIT_FAILURE,
 		     _("error: swap area needs to be at least %ld KiB"),
-		     (long)(MIN_GOODPAGES * pagesize / 1024));
-	if (PAGES > UINT32_MAX) {
+		     (long)(MIN_GOODPAGES * ctl.pagesize / 1024));
+	if (ctl.nr_pages > UINT32_MAX) {
 		/* true when swap is bigger than 17.59 terabytes */
-		PAGES = UINT32_MAX;
+		ctl.nr_pages = UINT32_MAX;
 		warnx(_("warning: truncating swap area to %llu KiB"),
-			PAGES * pagesize / 1024);
+			ctl.nr_pages * ctl.pagesize / 1024);
 	}
 
-	if (is_mounted(device_name))
+	if (is_mounted(ctl.device_name))
 		errx(EXIT_FAILURE, _("error: "
 			"%s is mounted; will not make swapspace"),
-			device_name);
+			ctl.device_name);
 
-	if (stat(device_name, &statbuf) < 0)
-		err(EXIT_FAILURE, _("stat failed %s"), device_name);
+	if (stat(ctl.device_name, &statbuf) < 0)
+		err(EXIT_FAILURE, _("stat failed %s"), ctl.device_name);
 	if (S_ISBLK(statbuf.st_mode))
-		DEV = open(device_name, O_RDWR | O_EXCL);
+		ctl.fd = open(ctl.device_name, O_RDWR | O_EXCL);
 	else
-		DEV = open(device_name, O_RDWR);
-	if (DEV < 0)
-		err(EXIT_FAILURE, _("cannot open %s"), device_name);
+		ctl.fd = open(ctl.device_name, O_RDWR);
+	if (ctl.fd < 0)
+		err(EXIT_FAILURE, _("cannot open %s"), ctl.device_name);
 
 	if (!S_ISBLK(statbuf.st_mode))
-		check=0;
-	else if (blkdev_is_misaligned(DEV))
-		warnx(_("warning: %s is misaligned"), device_name);
+		ctl.check = 0;
+	else if (blkdev_is_misaligned(ctl.fd))
+		warnx(_("warning: %s is misaligned"), ctl.device_name);
 
-	if (check)
-		check_blocks();
+	if (ctl.check)
+		check_blocks(&ctl);
 
-	wipe_device(DEV, device_name, force);
+	wipe_device(ctl.fd, ctl.device_name, ctl.force);
 
-	hdr = (struct swap_header_v1_2 *) signature_page;
-	hdr->version = version;
-	hdr->last_page = PAGES - 1;
-	hdr->nr_badpages = badpages;
+	ctl.hdr = (struct swap_header_v1_2 *) ctl.signature_page;
+	ctl.hdr->version = version;
+	ctl.hdr->last_page = ctl.nr_pages - 1;
+	ctl.hdr->nr_badpages = ctl.badpages;
 
-	if (badpages > PAGES - MIN_GOODPAGES)
+	if ((ctl.nr_pages - MIN_GOODPAGES) < ctl.badpages)
 		errx(EXIT_FAILURE, _("Unable to set up swap-space: unreadable"));
 
-	goodpages = PAGES - badpages - 1;
+	goodpages = ctl.nr_pages - ctl.badpages - 1;
 	printf(_("Setting up swapspace version %d, size = %llu KiB\n"),
-		version, goodpages * pagesize / 1024);
+		version, goodpages * ctl.pagesize / 1024);
 
-	write_signature(SWAP_SIGNATURE);
-	write_uuid_and_label(uuid, opt_label);
+	write_signature(&ctl);
+	write_uuid_and_label(&ctl);
 
 	offset = 1024;
-	if (lseek(DEV, offset, SEEK_SET) != offset)
+	if (lseek(ctl.fd, offset, SEEK_SET) != offset)
 		errx(EXIT_FAILURE, _("unable to rewind swap-device"));
-	if (write_all(DEV, (char *) signature_page + offset,
-				    pagesize - offset) == -1)
+	if (write_all(ctl.fd, (char *) ctl.signature_page + offset,
+				    ctl.pagesize - offset) == -1)
 		err(EXIT_FAILURE,
 			_("%s: unable to write signature page"),
-			device_name);
+			ctl.device_name);
 
 #ifdef HAVE_LIBSELINUX
 	if (S_ISREG(statbuf.st_mode) && is_selinux_enabled() > 0) {
@@ -481,12 +482,12 @@ main(int argc, char **argv) {
 		security_context_t oldcontext;
 		context_t newcontext;
 
-		if (fgetfilecon(DEV, &oldcontext) < 0) {
+		if (fgetfilecon(ctl.fd, &oldcontext) < 0) {
 			if (errno != ENODATA)
 				err(EXIT_FAILURE,
 					_("%s: unable to obtain selinux file label"),
-					device_name);
-			if (matchpathcon(device_name, statbuf.st_mode, &oldcontext))
+					ctl.device_name);
+			if (matchpathcon(ctl.device_name, statbuf.st_mode, &oldcontext))
 				errx(EXIT_FAILURE, _("unable to matchpathcon()"));
 		}
 		if (!(newcontext = context_new(oldcontext)))
@@ -497,9 +498,9 @@ main(int argc, char **argv) {
 		context_string = context_str(newcontext);
 
 		if (strcmp(context_string, oldcontext)!=0) {
-			if (fsetfilecon(DEV, context_string))
+			if (fsetfilecon(ctl.fd, context_string))
 				err(EXIT_FAILURE, _("unable to relabel %s to %s"),
-						device_name, context_string);
+						ctl.device_name, context_string);
 		}
 		context_free(newcontext);
 		freecon(oldcontext);
@@ -510,7 +511,7 @@ main(int argc, char **argv) {
 	 * is not actually on disk. (This is a kernel bug.)
 	 * The fsync() in close_fd() will take care of writing.
 	 */
-	if (close_fd(DEV) != 0)
+	if (close_fd(ctl.fd) != 0)
 		err(EXIT_FAILURE, _("write failed"));
 	return EXIT_SUCCESS;
 }
-- 
2.1.3

[PATCH 6/8] mkswap: make remaining functions to take control structure as argument

[Sami Kerola]

The wipe_device() and new_prober().

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

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index 7f41428..7f60013 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -225,31 +225,31 @@ get_size(const struct mkswap_control *ctl)
 
 #ifdef HAVE_LIBBLKID
 static blkid_probe
-new_prober(int fd)
+new_prober(const struct mkswap_control *ctl)
 {
 	blkid_probe pr = blkid_new_probe();
 	if (!pr)
 		errx(EXIT_FAILURE, _("unable to alloc new libblkid probe"));
-	if (blkid_probe_set_device(pr, fd, 0, 0))
+	if (blkid_probe_set_device(pr, ctl->fd, 0, 0))
 		errx(EXIT_FAILURE, _("unable to assign device to libblkid probe"));
 	return pr;
 }
 #endif
 
 static void
-wipe_device(int fd, const char *devname, int force)
+wipe_device(struct mkswap_control *ctl)
 {
 	char *type = NULL;
 	int zap = 1;
 #ifdef HAVE_LIBBLKID
 	blkid_probe pr = NULL;
 #endif
-	if (!force) {
-		if (lseek(fd, 0, SEEK_SET) != 0)
+	if (!ctl->force) {
+		if (lseek(ctl->fd, 0, SEEK_SET) != 0)
 			errx(EXIT_FAILURE, _("unable to rewind swap-device"));
 
 #ifdef HAVE_LIBBLKID
-		pr = new_prober(fd);
+		pr = new_prober(ctl);
 		blkid_probe_enable_partitions(pr, 1);
 		blkid_probe_enable_superblocks(pr, 0);
 
@@ -271,18 +271,18 @@ wipe_device(int fd, const char *devname, int force)
 		 */
 		char buf[1024];
 
-		if (lseek(fd, 0, SEEK_SET) != 0)
+		if (lseek(ctl->fd, 0, SEEK_SET) != 0)
 			errx(EXIT_FAILURE, _("unable to rewind swap-device"));
 
 		memset(buf, 0, sizeof(buf));
-		if (write_all(fd, buf, sizeof(buf)))
+		if (write_all(ctl->fd, buf, sizeof(buf)))
 			errx(EXIT_FAILURE, _("unable to erase bootbits sectors"));
 #ifdef HAVE_LIBBLKID
 		/*
 		 * Wipe rest of the device
 		 */
 		if (!pr)
-			pr = new_prober(fd);
+			pr = new_prober(ctl);
 
 		blkid_probe_enable_superblocks(pr, 1);
 		blkid_probe_enable_partitions(pr, 0);
@@ -292,13 +292,13 @@ wipe_device(int fd, const char *devname, int force)
 			const char *data = NULL;
 
 			if (blkid_probe_lookup_value(pr, "TYPE", &data, NULL) == 0 && data)
-				warnx(_("%s: warning: wiping old %s signature."), devname, data);
+				warnx(_("%s: warning: wiping old %s signature."), ctl->device_name, data);
 			blkid_do_wipe(pr, 0);
 		}
 #endif
 	} else {
 		warnx(_("%s: warning: don't erase bootbits sectors"),
-			devname);
+			ctl->device_name);
 		if (type)
 			fprintf(stderr, _("        (%s partition table detected). "), type);
 		else
@@ -450,7 +450,7 @@ main(int argc, char **argv) {
 	if (ctl.check)
 		check_blocks(&ctl);
 
-	wipe_device(ctl.fd, ctl.device_name, ctl.force);
+	wipe_device(&ctl);
 
 	ctl.hdr = (struct swap_header_v1_2 *) ctl.signature_page;
 	ctl.hdr->version = version;
-- 
2.1.3

[PATCH 7/8] mkswap: set variable only when it's value is known

[Sami Kerola]

Avoid updating ctl->pagesize twice when user does specify page size.

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

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index 7f60013..280d890 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -90,23 +90,21 @@ struct mkswap_control {
 static void
 init_signature_page(struct mkswap_control *ctl)
 {
-
-	int kernel_pagesize = ctl->pagesize = getpagesize();
+	const int kernel_pagesize = getpagesize();
 
 	if (ctl->user_pagesize) {
 		if (ctl->user_pagesize < 0 || !is_power_of_2(ctl->user_pagesize) ||
 		    (size_t) ctl->user_pagesize < sizeof(struct swap_header_v1_2) + 10)
 			errx(EXIT_FAILURE,
-				_("Bad user-specified page size %u"),
-				ctl->user_pagesize);
-		ctl->pagesize = ctl->user_pagesize;
-	}
-
-	if (ctl->user_pagesize && ctl->user_pagesize != kernel_pagesize)
-		warnx(_("Using user-specified page size %d, "
+			     _("Bad user-specified page size %u"),
+			       ctl->user_pagesize);
+		if (ctl->user_pagesize != kernel_pagesize)
+			warnx(_("Using user-specified page size %d, "
 				"instead of the system value %d"),
 				ctl->pagesize, kernel_pagesize);
-
+		ctl->pagesize = ctl->user_pagesize;
+	} else
+		ctl->pagesize = kernel_pagesize;
 	ctl->signature_page = (unsigned long *) xcalloc(1, ctl->pagesize);
 }
 
-- 
2.1.3

[PATCH 8/8] mkswap: various minor improvement

[Sami Kerola]

Use correct data type in page_bad(), and add information to error message
how many bad pages were seen.

In check_blocks() move initialization to variable introduction, fix typo,
and avoid memset() when array initializer can do the job.

In main() use correct initializer for pointer.  Move swap file specific
actions to same location, and add warning when request too --check is
dismissed by command.  At the end remove momory leak.

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

diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index 280d890..71f62b4 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -167,13 +167,13 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
 }
 
 static void
-page_bad(struct mkswap_control *ctl, int page)
+page_bad(struct mkswap_control *ctl, unsigned int page)
 {
 	struct swap_header_v1_2 *p = (struct swap_header_v1_2 *)ctl->signature_page;
 	const unsigned long max_badpages = (ctl->pagesize - 1024 - 128 * sizeof(int) - 10) / sizeof(int);
 
 	if (ctl->badpages == max_badpages)
-		errx(EXIT_FAILURE, _("too many bad pages"));
+		errx(EXIT_FAILURE, _("too many bad pages: %lu"), max_badpages);
 	p->badpages[ctl->badpages] = page;
 	ctl->badpages++;
 }
@@ -181,12 +181,11 @@ page_bad(struct mkswap_control *ctl, int page)
 static void
 check_blocks(struct mkswap_control *ctl)
 {
-	unsigned int current_page;
+	unsigned int current_page = 0;
 	int do_seek = 1;
 	char *buffer;
 
 	buffer = xmalloc(ctl->pagesize);
-	current_page = 0;
 	while (current_page < ctl->nr_pages) {
 		ssize_t rc;
 
@@ -265,14 +264,13 @@ wipe_device(struct mkswap_control *ctl)
 
 	if (zap) {
 		/*
-		 * Wipe boodbits
+		 * Wipe bootbits
 		 */
-		char buf[1024];
+		char buf[1024] = { '\0' };
 
 		if (lseek(ctl->fd, 0, SEEK_SET) != 0)
 			errx(EXIT_FAILURE, _("unable to rewind swap-device"));
 
-		memset(buf, 0, sizeof(buf));
 		if (write_all(ctl->fd, buf, sizeof(buf)))
 			errx(EXIT_FAILURE, _("unable to erase bootbits sectors"));
 #ifdef HAVE_LIBBLKID
@@ -317,7 +315,7 @@ main(int argc, char **argv) {
 	unsigned long long sz;
 	off_t offset;
 	int version = SWAP_VERSION;
-	char *block_count = 0;
+	char *block_count = NULL;
 #ifdef HAVE_LIBUUID
 	const char *opt_uuid = NULL;
 	uuid_t uuid_dat;
@@ -435,16 +433,19 @@ main(int argc, char **argv) {
 		err(EXIT_FAILURE, _("stat failed %s"), ctl.device_name);
 	if (S_ISBLK(statbuf.st_mode))
 		ctl.fd = open(ctl.device_name, O_RDWR | O_EXCL);
-	else
+	else {
+		if (ctl.check) {
+			ctl.check = 0;
+			warnx(_("warning: checking bad blocks from swap file is not supported: %s"),
+				ctl.device_name);
+		}
 		ctl.fd = open(ctl.device_name, O_RDWR);
+	}
 	if (ctl.fd < 0)
 		err(EXIT_FAILURE, _("cannot open %s"), ctl.device_name);
-
-	if (!S_ISBLK(statbuf.st_mode))
-		ctl.check = 0;
-	else if (blkdev_is_misaligned(ctl.fd))
-		warnx(_("warning: %s is misaligned"), ctl.device_name);
-
+	if (S_ISBLK(statbuf.st_mode))
+		if (blkdev_is_misaligned(ctl.fd))
+			warnx(_("warning: %s is misaligned"), ctl.device_name);
 	if (ctl.check)
 		check_blocks(&ctl);
 
@@ -473,7 +474,7 @@ main(int argc, char **argv) {
 		err(EXIT_FAILURE,
 			_("%s: unable to write signature page"),
 			ctl.device_name);
-
+	free(ctl.signature_page);
 #ifdef HAVE_LIBSELINUX
 	if (S_ISREG(statbuf.st_mode) && is_selinux_enabled() > 0) {
 		security_context_t context_string;
-- 
2.1.3

Re: [PATCH 0/8] pull: mostly mkswap and one swapoff chanage

[Karel Zak]

On Sun, Nov 02, 2014 at 08:26:23PM +0000, Sami Kerola wrote:
>   mkswap: remove system architecture specific max swap size checks
>   mkswap: remove unnecessary size check
>   mkswap: use err() rather than perror() && exit()
>   mkswap: add struct mkswap_control to remove global variables
>   mkswap: make remaining functions to take control structure as argument
>   mkswap: set variable only when it's value is known
>   mkswap: various minor improvement

merged. I'll probably cleanup the mkswap code too.

>  disk-utils/mkswap.c     | 435 ++++++++++++++++--------------------------------

 Thanks! :-)

    Karel

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