[PATCH mtd-utils 4/7] misc-utils: flashcp: simplify logging

Brandon Maier brandon.maier at collins.com
Wed Nov 2 15:47:54 PDT 2022


Most of the uses of log_printf fall into two styles

> if (flags & FLAG_VERBOSE)
>     log_printf(LOG_NORMAL, ...);

or

> log_printf(LOG_ERROR, ...)
> exit(EXIT_FAILURE);

Replace them with log_verbose and log_failure respectively.

Signed-off-by: Brandon Maier <brandon.maier at collins.com>
---
 misc-utils/flashcp.c | 173 +++++++++++++++++++------------------------
 1 file changed, 77 insertions(+), 96 deletions(-)

diff --git a/misc-utils/flashcp.c b/misc-utils/flashcp.c
index 8cb0dbb..9e669b4 100644
--- a/misc-utils/flashcp.c
+++ b/misc-utils/flashcp.c
@@ -48,7 +48,7 @@
 /* for debugging purposes only */
 #ifdef DEBUG
 #undef DEBUG
-#define DEBUG(fmt,args...) { log_printf (LOG_ERROR,"%d: ",__LINE__); log_printf (LOG_ERROR,fmt,## args); }
+#define DEBUG(fmt,args...) { fprintf (stderr,"%d: ",__LINE__); fprintf (stderr,fmt,## args); }
 #else
 #undef DEBUG
 #define DEBUG(fmt,args...)
@@ -62,7 +62,6 @@
 
 /* cmd-line flags */
 #define FLAG_NONE		0x00
-#define FLAG_VERBOSE	0x01
 #define FLAG_HELP		0x02
 #define FLAG_FILENAME	0x04
 #define FLAG_DEVICE		0x08
@@ -73,21 +72,34 @@
 #define LOG_NORMAL	1
 #define LOG_ERROR	2
 
-static void log_printf (int level,const char *fmt, ...)
+static NORETURN void log_failure (const char *fmt, ...)
 {
-	FILE *fp = level == LOG_NORMAL ? stdout : stderr;
 	va_list ap;
 	va_start (ap,fmt);
-	vfprintf (fp,fmt,ap);
+	vfprintf (stderr,fmt,ap);
 	va_end (ap);
-	fflush (fp);
+	fflush (stderr);
+
+	exit (EXIT_FAILURE);
 }
 
-static NORETURN void showusage(bool error)
+static int verbose = 0;
+static void log_verbose (const char *fmt, ...)
 {
-	int level = error ? LOG_ERROR : LOG_NORMAL;
+	va_list ap;
 
-	log_printf (level,
+	if (!verbose)
+		return;
+
+	va_start (ap,fmt);
+	vfprintf (stdout,fmt,ap);
+	va_end (ap);
+	fflush (stdout);
+}
+
+static NORETURN void showusage(bool error)
+{
+	fprintf (error ? stderr : stdout,
 			"\n"
 			"Flash Copy - Written by Abraham van der Merwe <abraham at 2d3d.co.za>\n"
 			"\n"
@@ -110,44 +122,42 @@ static NORETURN void showusage(bool error)
 
 static int safe_open (const char *pathname,int flags)
 {
+	const char *access = "unknown";
 	int fd;
 
 	fd = open (pathname,flags);
 	if (fd < 0)
 	{
-		log_printf (LOG_ERROR,"While trying to open %s",pathname);
 		if (flags & O_RDWR)
-			log_printf (LOG_ERROR," for read/write access");
+			access = "read/write";
 		else if (flags & O_RDONLY)
-			log_printf (LOG_ERROR," for read access");
+			access = "read";
 		else if (flags & O_WRONLY)
-			log_printf (LOG_ERROR," for write access");
-		log_printf (LOG_ERROR,": %m\n");
-		exit (EXIT_FAILURE);
+			access = "write";
+
+		log_failure ("While trying to open %s for %s access: %m\n",pathname,access);
 	}
 
 	return (fd);
 }
 
-static void safe_read (int fd,const char *filename,void *buf,size_t count,bool verbose)
+static void safe_read (int fd,const char *filename,void *buf,size_t count)
 {
 	ssize_t result;
 
 	result = read (fd,buf,count);
 	if (count != result)
 	{
-		if (verbose) log_printf (LOG_NORMAL,"\n");
+		log_verbose ("\n");
 		if (result < 0)
 		{
-			log_printf (LOG_ERROR,"While reading data from %s: %m\n",filename);
-			exit (EXIT_FAILURE);
+			log_failure("While reading data from %s: %m\n",filename);
 		}
-		log_printf (LOG_ERROR,"Short read count returned while reading from %s\n",filename);
-		exit (EXIT_FAILURE);
+		log_failure("Short read count returned while reading from %s\n",filename);
 	}
 }
 
-static void safe_write (int fd,const void *buf,size_t count,size_t written,unsigned long long to_write,const char *device,bool verbose)
+static void safe_write (int fd,const void *buf,size_t count,size_t written,unsigned long long to_write,const char *device)
 {
 	ssize_t result;
 
@@ -155,18 +165,14 @@ static void safe_write (int fd,const void *buf,size_t count,size_t written,unsig
 	result = write (fd,buf,count);
 	if (count != result)
 	{
-		if (verbose) log_printf (LOG_NORMAL,"\n");
+		log_verbose ("\n");
 		if (result < 0)
 		{
-			log_printf (LOG_ERROR,
-					"While writing data to 0x%.8lx-0x%.8lx on %s: %m\n",
+			log_failure("While writing data to 0x%.8lx-0x%.8lx on %s: %m\n",
 					written,written + count,device);
-			exit (EXIT_FAILURE);
 		}
-		log_printf (LOG_ERROR,
-				"Short write count returned while writing to x%.8zx-0x%.8zx on %s: %zu/%llu bytes written to flash\n",
+		log_failure("Short write count returned while writing to x%.8zx-0x%.8zx on %s: %zu/%llu bytes written to flash\n",
 				written,written + count,device,written + result,to_write);
-		exit (EXIT_FAILURE);
 	}
 }
 
@@ -177,8 +183,7 @@ static off_t safe_lseek (int fd,off_t offset,int whence,const char *filename)
 	off = lseek (fd,offset,whence);
 	if (off < 0)
 	{
-		log_printf (LOG_ERROR,"While seeking on %s: %m\n",filename);
-		exit (EXIT_FAILURE);
+		log_failure("While seeking on %s: %m\n",filename);
 	}
 
 	return off;
@@ -189,15 +194,13 @@ static void safe_rewind (int fd,const char *filename)
 	safe_lseek(fd,0L,SEEK_SET,filename);
 }
 
-static void safe_memerase (int fd,const char *device,struct erase_info_user *erase,bool verbose)
+static void safe_memerase (int fd,const char *device,struct erase_info_user *erase)
 {
 	if (ioctl (fd,MEMERASE,erase) < 0)
 	{
-		if (verbose) log_printf (LOG_NORMAL,"\n");
-		log_printf (LOG_ERROR,
-				"While erasing blocks 0x%.8x-0x%.8x on %s: %m\n",
+		log_verbose ("\n");
+		log_failure("While erasing blocks 0x%.8x-0x%.8x on %s: %m\n",
 				(unsigned int) erase->start,(unsigned int) (erase->start + erase->length),device);
-		exit (EXIT_FAILURE);
 	}
 }
 
@@ -250,7 +253,7 @@ int main (int argc,char *argv[])
 				DEBUG("Got FLAG_HELP\n");
 				break;
 			case 'v':
-				flags |= FLAG_VERBOSE;
+				verbose = 1;
 				DEBUG("Got FLAG_VERBOSE\n");
 				break;
 			case 'p':
@@ -290,24 +293,17 @@ int main (int argc,char *argv[])
 	if (ioctl (dev_fd,MEMGETINFO,&mtd) < 0)
 	{
 		DEBUG("ioctl(): %m\n");
-		log_printf (LOG_ERROR,"This doesn't seem to be a valid MTD flash device!\n");
-		exit (EXIT_FAILURE);
+		log_failure("This doesn't seem to be a valid MTD flash device!\n");
 	}
 
 	/* get some info about the file we want to copy */
 	fil_fd = safe_open (filename,O_RDONLY);
 	if (fstat (fil_fd,&filestat) < 0)
-	{
-		log_printf (LOG_ERROR,"While trying to get the file status of %s: %m\n",filename);
-		exit (EXIT_FAILURE);
-	}
+		log_failure("While trying to get the file status of %s: %m\n",filename);
 
 	/* does it fit into the device/partition? */
 	if (filestat.st_size > mtd.size)
-	{
-		log_printf (LOG_ERROR,"%s won't fit into %s!\n",filename,device);
-		exit (EXIT_FAILURE);
-	}
+		log_failure("%s won't fit into %s!\n",filename,device);
 
 	/* diff block flashcp */
 	if (flags & FLAG_PARTITION)
@@ -333,24 +329,24 @@ int main (int argc,char *argv[])
 		erase.length *= mtd.erasesize;
 	}
 
-	if (flags & FLAG_VERBOSE)
+	if (verbose)
 	{
 		/* if the user wants verbose output, erase 1 block at a time and show him/her what's going on */
 		int blocks = erase.length / mtd.erasesize;
 		erase.length = mtd.erasesize;
-		log_printf (LOG_NORMAL,"Erasing blocks: 0/%d (0%%)",blocks);
+		log_verbose ("Erasing blocks: 0/%d (0%%)",blocks);
 		for (i = 1; i <= blocks; i++)
 		{
-			log_printf (LOG_NORMAL,"\rErasing blocks: %d/%d (%d%%)",i,blocks,PERCENTAGE (i,blocks));
-			safe_memerase(dev_fd,device,&erase,flags & FLAG_VERBOSE);
+			log_verbose ("\rErasing blocks: %d/%d (%d%%)",i,blocks,PERCENTAGE (i,blocks));
+			safe_memerase(dev_fd,device,&erase);
 			erase.start += mtd.erasesize;
 		}
-		log_printf (LOG_NORMAL,"\rErasing blocks: %d/%d (100%%)\n",blocks,blocks);
+		log_verbose ("\rErasing blocks: %d/%d (100%%)\n",blocks,blocks);
 	}
 	else
 	{
 		/* if not, erase the whole chunk in one shot */
-		safe_memerase(dev_fd,device,&erase,flags & FLAG_VERBOSE);
+		safe_memerase(dev_fd,device,&erase);
 	}
 	DEBUG("Erased %u / %luk bytes\n",erase.length,filestat.st_size);
 
@@ -358,33 +354,30 @@ int main (int argc,char *argv[])
 	 * write the entire file to flash *
 	 **********************************/
 
-	if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"Writing data: 0k/%lluk (0%%)",KB ((unsigned long long)filestat.st_size));
+	log_verbose ("Writing data: 0k/%lluk (0%%)",KB ((unsigned long long)filestat.st_size));
 	size = filestat.st_size;
 	i = BUFSIZE;
 	written = 0;
 	while (size)
 	{
 		if (size < BUFSIZE) i = size;
-		if (flags & FLAG_VERBOSE)
-			log_printf (LOG_NORMAL,"\rWriting data: %dk/%lluk (%llu%%)",
-					KB (written + i),
-					KB ((unsigned long long)filestat.st_size),
-					PERCENTAGE (written + i,(unsigned long long)filestat.st_size));
+		log_verbose ("\rWriting data: %dk/%lluk (%llu%%)",
+				KB (written + i),
+				KB ((unsigned long long)filestat.st_size),
+				PERCENTAGE (written + i,(unsigned long long)filestat.st_size));
 
 		/* read from filename */
-		safe_read (fil_fd,filename,src,i,flags & FLAG_VERBOSE);
+		safe_read (fil_fd,filename,src,i);
 
 		/* write to device */
-		safe_write(dev_fd,src,i,written,(unsigned long long)filestat.st_size,device,flags & FLAG_VERBOSE);
+		safe_write(dev_fd,src,i,written,(unsigned long long)filestat.st_size,device);
 
 		written += i;
 		size -= i;
 	}
-	if (flags & FLAG_VERBOSE)
-		log_printf (LOG_NORMAL,
-				"\rWriting data: %lluk/%lluk (100%%)\n",
-				KB ((unsigned long long)filestat.st_size),
-				KB ((unsigned long long)filestat.st_size));
+	log_verbose ("\rWriting data: %lluk/%lluk (100%%)\n",
+			KB ((unsigned long long)filestat.st_size),
+			KB ((unsigned long long)filestat.st_size));
 	DEBUG("Wrote %d / %lluk bytes\n",written,(unsigned long long)filestat.st_size);
 
 	/**********************************
@@ -396,40 +389,32 @@ int main (int argc,char *argv[])
 	size = filestat.st_size;
 	i = BUFSIZE;
 	written = 0;
-	if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"Verifying data: 0k/%lluk (0%%)",KB ((unsigned long long)filestat.st_size));
+	log_verbose ("Verifying data: 0k/%lluk (0%%)",KB ((unsigned long long)filestat.st_size));
 	while (size)
 	{
 		if (size < BUFSIZE) i = size;
-		if (flags & FLAG_VERBOSE)
-			log_printf (LOG_NORMAL,
-					"\rVerifying data: %luk/%lluk (%llu%%)",
-					KB (written + i),
-					KB ((unsigned long long)filestat.st_size),
-					PERCENTAGE (written + i,(unsigned long long)filestat.st_size));
+		log_verbose ("\rVerifying data: %luk/%lluk (%llu%%)",
+				KB (written + i),
+				KB ((unsigned long long)filestat.st_size),
+				PERCENTAGE (written + i,(unsigned long long)filestat.st_size));
 
 		/* read from filename */
-		safe_read (fil_fd,filename,src,i,flags & FLAG_VERBOSE);
+		safe_read (fil_fd,filename,src,i);
 
 		/* read from device */
-		safe_read (dev_fd,device,dest,i,flags & FLAG_VERBOSE);
+		safe_read (dev_fd,device,dest,i);
 
 		/* compare buffers */
 		if (memcmp (src,dest,i))
-		{
-			log_printf (LOG_ERROR,
-					"File does not seem to match flash data. First mismatch at 0x%.8zx-0x%.8zx\n",
+			log_failure("File does not seem to match flash data. First mismatch at 0x%.8zx-0x%.8zx\n",
 					written,written + i);
-			exit (EXIT_FAILURE);
-		}
 
 		written += i;
 		size -= i;
 	}
-	if (flags & FLAG_VERBOSE)
-		log_printf (LOG_NORMAL,
-				"\rVerifying data: %lluk/%lluk (100%%)\n",
-				KB ((unsigned long long)filestat.st_size),
-				KB ((unsigned long long)filestat.st_size));
+	log_verbose ("\rVerifying data: %lluk/%lluk (100%%)\n",
+			KB ((unsigned long long)filestat.st_size),
+			KB ((unsigned long long)filestat.st_size));
 	DEBUG("Verified %d / %lluk bytes\n",written,(unsigned long long)filestat.st_size);
 
 	exit (EXIT_SUCCESS);
@@ -451,22 +436,18 @@ DIFF_BLOCKS:
 	int blocks = erase.length / mtd.erasesize;
 	erase.length = mtd.erasesize;
 
-	if (flags & FLAG_VERBOSE)
-		log_printf (LOG_NORMAL,
-				"\rProcessing blocks: 0/%d (%d%%)", blocks, PERCENTAGE (0,blocks));
+	log_verbose ("\rProcessing blocks: 0/%d (%d%%)", blocks, PERCENTAGE (0,blocks));
 	for (int s = 1; s <= blocks; s++)
 	{
 		if (size < mtd.erasesize) i = size;
-		if (flags & FLAG_VERBOSE)
-			log_printf (LOG_NORMAL,
-					"\rProcessing blocks: %d/%d (%d%%)", s, blocks, PERCENTAGE (s,blocks));
+		log_verbose ("\rProcessing blocks: %d/%d (%d%%)", s, blocks, PERCENTAGE (s,blocks));
 
 		/* read from filename */
-		safe_read (fil_fd,filename,src,i,flags & FLAG_VERBOSE);
+		safe_read (fil_fd,filename,src,i);
 
 		/* read from device */
 		current_dev_block = safe_lseek(dev_fd, 0, SEEK_CUR, device);
-		safe_read (dev_fd,device,dest,i,flags & FLAG_VERBOSE);
+		safe_read (dev_fd,device,dest,i);
 
 		/* compare buffers, if not the same, erase and write the block */
 		if (memcmp (src,dest,i))
@@ -474,11 +455,11 @@ DIFF_BLOCKS:
 			diffBlock++;
 			/* erase block */
 			safe_lseek(dev_fd, current_dev_block, SEEK_SET, device);
-			safe_memerase(dev_fd,device,&erase,flags & FLAG_VERBOSE);
+			safe_memerase(dev_fd,device,&erase);
 
 			/* write to device */
 			safe_lseek(dev_fd, current_dev_block, SEEK_SET, device);
-			safe_write(dev_fd,src,i,written,(unsigned long long)filestat.st_size,device,flags & FLAG_VERBOSE);
+			safe_write(dev_fd,src,i,written,(unsigned long long)filestat.st_size,device);
 		}
 
 		erase.start += i;
@@ -486,7 +467,7 @@ DIFF_BLOCKS:
 		size -= i;
 	}
 
-	if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL, "\ndiff blocks: %d\n", diffBlock);
+	log_verbose ("\ndiff blocks: %d\n", diffBlock);
 
 	exit (EXIT_SUCCESS);
 }
-- 
2.38.1




More information about the linux-mtd mailing list