Enable stdin verification and display correct number of bytes written/verified

Counting the number of bytes written to a memory and/or verified is not
trivial owing to potential holes in the input file and to potential trailing
0xff bytes in flash memory that are not written per default (but see -A). The
new function memstats(), which is best called just after an input file has
been read into mem->buf/mem->tags, computes the right number of bytes written
and allows easy computation of the number of bytes verified.

This commit also changes the strategy for the default verification after
writing to a chip memory, so that the input file only needs reading once thus
enabling successful verification of stdin input files.

Other, minor changes:
 - Improving the grammar of AVRDUDE output, eg, 1 byte written instead of
   1 bytes written
 - Better description of the input file structure in terms of its sections,
   the interval it spans, the number  of pages, the number of padding bytes
   in pages, and the number of actually cut off trailing 0xff bytes for flash
 - Printing <stdin> or <stdout> instead of - in the -U routines
 - Option -V no longer needs to be specified before option -U in order to work

As an aside this commit also provides useful helper functions for printing
plural(), inname(), outname() and interval() all of which return strings fit
for printing.

$ avrdude -qp ATmega2560 -c usbtiny -U blink-mega2560+lext-test.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e9801 (probably m2560)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: input file blink-mega2560+lext-test.hex auto detected as Intel Hex
avrdude: reading input file blink-mega2560+lext-test.hex for flash
         with 1346 bytes in 4 sections within [0, 0x3106d]
         using 7 pages and 446 pad bytes
avrdude: writing 1346 bytes flash ...
avrdude: 1346 bytes of flash written
avrdude: verifying flash memory against blink-mega2560+lext-test.hex
avrdude: 1346 bytes of flash verified

avrdude done.  Thank you.

$ avrdude -qp ATmega328P -c usb-bub-ii -U sketch-ending-in-ff.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: input file sketch-ending-in-ff.hex auto detected as Intel Hex
avrdude: reading input file sketch-ending-in-ff.hex for flash
         with 2160 bytes in 1 section within [0, 0x888]
         using 17 pages and 16 pad bytes, cutting off 25 trailing 0xff bytes
avrdude: writing 2160 bytes flash ...
avrdude: 2160 bytes of flash written
avrdude: verifying flash memory against sketch-ending-in-ff.hex
avrdude: 2185 bytes of flash verified

avrdude done.  Thank you.

$ echo "Hello, world..." | avrdude -qp ATmega328P -c ... -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: reading input file <stdin> for eeprom
avrdude: writing 16 bytes eeprom ...
avrdude: 16 bytes of eeprom written
avrdude: verifying eeprom memory against <stdin>
avrdude: 16 bytes of eeprom verified

avrdude done.  Thank you.

src/libavrdude.h

@@ -888,6 +888,7 @@ enum updateflags {
   UF_NONE = 0,
   UF_NOWRITE = 1,
   UF_AUTO_ERASE = 2,
+  UF_VERIFY = 4,
 };
 
 
@@ -898,6 +899,17 @@ typedef struct update_t {
   int    format;
 } UPDATE;
 
+typedef struct {                // File reads for flash can exclude trailing 0xff, which are cut off
+  int nbytes,                   // Number of bytes set including 0xff but excluding cut off, trailing 0xff
+      nsections,                // Number of consecutive sections in source excluding cut off, trailing 0xff
+      npages,                   // Number of memory pages needed excluding pages solely with trailing 0xff
+      nfill,                    // Number of fill bytes to make up full pages that are needed
+      ntrailing,                // Number of trailing 0xff in source
+      firstaddr,                // First address set in [0, mem->size-1]
+      lastaddr;                 // Highest address set by input file
+} Filestats;
+
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -910,6 +922,14 @@ extern void free_update(UPDATE * upd);
 extern int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd,
 		 enum updateflags flags);
 
+extern int memstats(struct avrpart *p, char *memtype, int size, Filestats *fsp);
+
+// Convenience functions for printing
+const char *plural(int x);
+const char *inname(const char *fn);
+const char *outname(const char *fn);
+const char *interval(int a, int b);
+
 #ifdef __cplusplus
 }
 #endif

src/main.c

@@ -237,7 +237,7 @@ static void cleanup_main(void)
 static void replace_backslashes(char *s)
 {
   // Replace all backslashes with forward slashes
-  for (int i = 0; i < strlen(s); i++) {
+  for (size_t i = 0; i < strlen(s); i++) {
     if (s[i] == '\\') {
       s[i] = '/';
     }
@@ -267,7 +267,6 @@ int main(int argc, char * argv [])
   int     calibrate;   /* 1=calibrate RC oscillator, 0=don't */
   char  * port;        /* device port (/dev/xxx) */
   int     terminal;    /* 1=enter terminal mode, 0=don't */
-  int     verify;      /* perform a verify operation */
   char  * exitspecs;   /* exit specs string from command line */
   char  * programmer;  /* programmer id */
   char  * partdesc;    /* part id */
@@ -284,7 +283,7 @@ int main(int argc, char * argv [])
   int     init_ok;     /* Device initialization worked well */
   int     is_open;     /* Device open succeeded */
   char  * logfile;     /* Use logfile rather than stderr for diagnostics */
-  enum updateflags uflags = UF_AUTO_ERASE; /* Flags for do_op() */
+  enum updateflags uflags = UF_AUTO_ERASE | UF_VERIFY; /* Flags for do_op() */
 
 #if !defined(WIN32)
   char  * homedir;
@@ -349,7 +348,6 @@ int main(int argc, char * argv [])
   p             = NULL;
   ovsigck       = 0;
   terminal      = 0;
-  verify        = 1;        /* on by default */
   quell_progress = 0;
   exitspecs     = NULL;
   pgm           = NULL;
@@ -525,12 +523,6 @@ int main(int argc, char * argv [])
           exit(1);
         }
         ladd(updates, upd);
-
-        if (verify && upd->op == DEVICE_WRITE) {
-          upd = dup_update(upd);
-          upd->op = DEVICE_VERIFY;
-          ladd(updates, upd);
-        }
         break;
 
       case 'v':
@@ -538,7 +530,7 @@ int main(int argc, char * argv [])
         break;
 
       case 'V':
-        verify = 0;
+        uflags &= ~UF_VERIFY;
         break;
 
       case 'x':

src/update.c

@@ -45,7 +45,7 @@ UPDATE * parse_op(char * s)
 
   i = 0;
   p = s;
-  while ((i < (sizeof(buf)-1) && *p && (*p != ':')))
+  while (i < (int) sizeof(buf)-1 && *p && *p != ':')
     buf[i++] = *p++;
   buf[i] = 0;
 
@@ -214,18 +214,124 @@ void free_update(UPDATE * u)
 }
 
 
+// Memory statistics considering holes after a file read returned size bytes
+int memstats(struct avrpart *p, char *memtype, int size, Filestats *fsp) {
+  Filestats ret = { 0 };
+  AVRMEM *mem = avr_locate_mem(p, memtype);
+
+  if(!mem) {
+    avrdude_message(MSG_INFO, "%s: %s %s undefined\n",
+      progname, p->desc, memtype);
+    return LIBAVRDUDE_GENERAL_FAILURE;
+  }
+
+  if(!mem->buf || !mem->tags) {
+    avrdude_message(MSG_INFO, "%s: %s %s is not set\n",
+      progname, p->desc, memtype);
+    return LIBAVRDUDE_GENERAL_FAILURE;
+  }
+
+  int pgsize = mem->page_size;
+  if(pgsize < 1)
+    pgsize = 1;
+
+  if(size < 0 || size > mem->size) {
+    avrdude_message(MSG_INFO, "%s: memstats() size %d at odds with %s %s size %d\n",
+      progname, size, p->desc, memtype, mem->size);
+    return LIBAVRDUDE_GENERAL_FAILURE;
+  }
+
+  ret.lastaddr = -1;
+  int firstset = 0, insection = 0;
+  // Scan all memory
+  for(int addr = 0; addr < mem->size; ) {
+    int pageset = 0;
+    // Go page by page
+    for(int pgi = 0; pgi < pgsize; pgi++, addr++) {
+      if(mem->tags[addr] & TAG_ALLOCATED) {
+        if(!firstset) {
+          firstset = 1;
+          ret.firstaddr = addr;
+        }
+        ret.lastaddr = addr;
+        // size can be smaller than tags suggest owing to flash trailing-0xff
+        if(addr < size) {
+          ret.nbytes++;
+          if(!pageset) {
+            pageset = 1;
+            ret.nfill += pgi;
+            ret.npages++;
+          }
+          if(!insection) {
+            insection = 1;
+            ret.nsections++;
+          }
+        } else {                // Now beyond size returned by input file read
+          ret.ntrailing++;
+          if(pageset)
+            ret.nfill++;
+        }
+      } else {                  // In a hole or beyond input file
+        insection = 0;
+        if(pageset)
+          ret.nfill++;
+      }
+    }
+  }
+
+  if(fsp)
+    *fsp = ret;
+
+  return LIBAVRDUDE_SUCCESS;
+}
+
+
+// Convenience functions for printing
+const char *plural(int x) {
+  return x==1? "": "s";
+}
+
+const char *inname(const char *fn) {
+  return !fn? "???": strcmp(fn, "-")? fn: "<stdin>";
+}
+
+const char *outname(const char *fn) {
+  return !fn? "???": strcmp(fn, "-")? fn: "<stdout>";
+}
+
+// Return sth like "[0, 0x1ff]"
+const char *interval(int a, int b) {
+  // Cyclic buffer for 20+ temporary interval strings each max 41 bytes at 64-bit int
+  static char space[20*41 + 80], *sp;
+  if(!sp || sp-space > (int) sizeof space - 80)
+    sp = space;
+
+  char *ret = sp;
+
+  sprintf(sp, a<16? "[%d": "[0x%x", a);
+  sp += strlen(sp);
+  sprintf(sp, b<16? ", %d]": ", 0x%x]", b);
+
+  // Advance beyond return string in temporary ring buffer
+  sp += strlen(sp)+1;
+
+  return ret;
+}
+
+
 int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags flags)
 {
   struct avrpart * v;
   AVRMEM * mem;
-  int size, vsize;
+  int size;
   int rc;
+  Filestats fs;
 
   mem = avr_locate_mem(p, upd->memtype);
   if (mem == NULL) {
-    avrdude_message(MSG_INFO, "\"%s\" memory type not defined for part \"%s\"\n",
+    avrdude_message(MSG_INFO, "%s memory type not defined for part %s\n",
       upd->memtype, p->desc);
-    return -1;
+    return LIBAVRDUDE_GENERAL_FAILURE;
   }
 
   AVRMEM_ALIAS * alias_mem = avr_find_memalias(p, mem);
@@ -235,167 +341,189 @@ int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags f
     strcat(alias_mem_desc, alias_mem->desc);
   }
   
-  if (upd->op == DEVICE_READ) {
+  switch (upd->op) {
-    /*
+  case DEVICE_READ:
-     * read out the specified device memory and write it to a file
+    // Read out the specified device memory and write it to a file
-     */
     if (upd->format == FMT_IMM) {
       avrdude_message(MSG_INFO,
-                      "%s: Invalid file format 'immediate' for output\n",
+        "%s: Invalid file format 'immediate' for output\n", progname);
-                      progname, upd->filename);
+      return LIBAVRDUDE_GENERAL_FAILURE;
-      return -1;
     }
-    if (quell_progress < 2) {
+    if (quell_progress < 2)
-      avrdude_message(MSG_INFO, "%s: reading %s%s memory:\n",
+      avrdude_message(MSG_INFO, "%s: reading %s%s memory ...\n",
         progname, mem->desc, alias_mem_desc);
-	  }
+
-    report_progress(0,1,"Reading");
+    report_progress(0, 1, "Reading");
     rc = avr_read(pgm, p, upd->memtype, 0);
+    report_progress(1, 1, NULL);
     if (rc < 0) {
       avrdude_message(MSG_INFO, "%s: failed to read all of %s%s memory, rc=%d\n",
         progname, mem->desc, alias_mem_desc, rc);
-      return -1;
+      return LIBAVRDUDE_GENERAL_FAILURE;
     }
-    report_progress(1,1,NULL);
     size = rc;
 
     if (quell_progress < 2) {
       if (rc == 0)
-        avrdude_message(MSG_INFO, "%s: Flash is empty, resulting file has no contents.\n",
+        avrdude_message(MSG_INFO, "%s: flash is empty, resulting file has no contents\n",
           progname);
-      avrdude_message(MSG_INFO, "%s: writing output file \"%s\"\n",
+      avrdude_message(MSG_INFO, "%s: writing output file %s\n",
-                      progname,
+        progname, outname(upd->filename));
-                      strcmp(upd->filename, "-")==0 ? "<stdout>" : upd->filename);
     }
     rc = fileio(FIO_WRITE, upd->filename, upd->format, p, upd->memtype, size);
     if (rc < 0) {
-      avrdude_message(MSG_INFO, "%s: write to file '%s' failed\n",
+      avrdude_message(MSG_INFO, "%s: write to file %s failed\n",
-              progname, upd->filename);
+        progname, outname(upd->filename));
-      return -1;
+      return LIBAVRDUDE_GENERAL_FAILURE;
-    }
-  }
-  else if (upd->op == DEVICE_WRITE) {
-    /*
-     * write the selected device memory using data from a file; first
-     * read the data from the specified file
-     */
-    if (quell_progress < 2) {
-      avrdude_message(MSG_INFO, "%s: reading input file \"%s\"\n",
-                      progname,
-                      strcmp(upd->filename, "-")==0 ? "<stdin>" : upd->filename);
     }
+    break;
+
+  case DEVICE_WRITE:
+    // Write the selected device memory using data from a file
+
     rc = fileio(FIO_READ, upd->filename, upd->format, p, upd->memtype, -1);
+    if (quell_progress < 2)
+      avrdude_message(MSG_INFO, "%s: reading input file %s for %s%s\n",
+        progname, inname(upd->filename), mem->desc, alias_mem_desc);
     if (rc < 0) {
-      avrdude_message(MSG_INFO, "%s: read from file '%s' failed\n",
+      avrdude_message(MSG_INFO, "%s: read from file %s failed\n",
-              progname, upd->filename);
+        progname, inname(upd->filename));
-      return -1;
+      return LIBAVRDUDE_GENERAL_FAILURE;
     }
     size = rc;
 
-    /*
+    if(memstats(p, upd->memtype, size, &fs) < 0)
-     * write the buffer contents to the selected memory type
+      return LIBAVRDUDE_GENERAL_FAILURE;
-     */
+
-    if (quell_progress < 2) {
+    if(quell_progress < 2) {
-      avrdude_message(MSG_INFO, "%s: writing %s%s (%d bytes):\n",
+      int level = fs.nsections > 1 || fs.firstaddr > 0 || fs.ntrailing? MSG_INFO: MSG_NOTICE;
-            progname, mem->desc, alias_mem_desc, size);
+
+      avrdude_message(level, "%*s with %d byte%s in %d section%s within %s\n",
+        (int) strlen(progname)+1, "",
+        fs.nbytes, plural(fs.nbytes),
+        fs.nsections, plural(fs.nsections),
+        interval(fs.firstaddr, fs.lastaddr));
+      if(mem->page_size > 1) {
+        avrdude_message(level, "%*s using %d page%s and %d pad byte%s",
+          (int) strlen(progname)+1, "",
+          fs.npages, plural(fs.npages),
+          fs.nfill, plural(fs.nfill));
+        if(fs.ntrailing)
+          avrdude_message(level, ", cutting off %d trailing 0xff byte%s",
+            fs.ntrailing, plural(fs.ntrailing));
+        avrdude_message(level, "\n");
+      }
     }
 
+    // Write the buffer contents to the selected memory type
+    if (quell_progress < 2)
+      avrdude_message(MSG_INFO, "%s: writing %d byte%s %s%s ...\n",
+        progname, fs.nbytes, plural(fs.nbytes), mem->desc, alias_mem_desc);
+
     if (!(flags & UF_NOWRITE)) {
-      report_progress(0,1,"Writing");
+      report_progress(0, 1, "Writing");
       rc = avr_write(pgm, p, upd->memtype, size, (flags & UF_AUTO_ERASE) != 0);
-      report_progress(1,1,NULL);
+      report_progress(1, 1, NULL);
-    }
+    } else {
-    else {
+      // Test mode: write to stdout in intel hex rather than to the chip
-      /*
-       * test mode, don't actually write to the chip, output the buffer
-       * to stdout in intel hex instead
-       */
       rc = fileio(FIO_WRITE, "-", FMT_IHEX, p, upd->memtype, size);
     }
 
     if (rc < 0) {
       avrdude_message(MSG_INFO, "%s: failed to write %s%s memory, rc=%d\n",
         progname, mem->desc, alias_mem_desc, rc);
-      return -1;
+      return LIBAVRDUDE_GENERAL_FAILURE;
     }
 
-    vsize = rc;
+    if (quell_progress < 2)
+      avrdude_message(MSG_INFO, "%s: %d byte%s of %s%s written\n",
+        progname, fs.nbytes, plural(fs.nbytes), mem->desc, alias_mem_desc);
 
-    if (quell_progress < 2) {
+    // Fall through for (default) auto verify, ie, unless -V was specified
-      avrdude_message(MSG_INFO, "%s: %d bytes of %s%s written\n", progname,
+    if (!(flags & UF_VERIFY))
-            vsize, mem->desc, alias_mem_desc);
+      break;
-    }
 
-  }
+  case DEVICE_VERIFY:
-  else if (upd->op == DEVICE_VERIFY) {
+    // Verify that the in memory file is the same as what is on the chip
-    /*
-     * verify that the in memory file (p->mem[AVR_M_FLASH|AVR_M_EEPROM])
-     * is the same as what is on the chip
-     */
     pgm->vfy_led(pgm, ON);
 
-    if (quell_progress < 2) {
+    int userverify = upd->op == DEVICE_VERIFY; // Explicit -U :v by user
-      avrdude_message(MSG_INFO, "%s: verifying %s%s memory against %s:\n",
-            progname, mem->desc, alias_mem_desc, upd->filename);
 
-      avrdude_message(MSG_NOTICE2, "%s: load data %s%s data from input file %s:\n",
+    if (quell_progress < 2) {
-            progname, mem->desc, alias_mem_desc, upd->filename);
+      avrdude_message(MSG_INFO, "%s: verifying %s%s memory against %s\n",
+        progname, mem->desc, alias_mem_desc, inname(upd->filename));
+
+      if (userverify)
+        avrdude_message(MSG_NOTICE, "%s: load %s%s data from input file %s\n",
+          progname, mem->desc, alias_mem_desc, inname(upd->filename));
     }
 
+    // No need to read file when fallen through from DEVICE_WRITE
+    if (userverify) {
       rc = fileio(FIO_READ_FOR_VERIFY, upd->filename, upd->format, p, upd->memtype, -1);
+
       if (rc < 0) {
-      avrdude_message(MSG_INFO, "%s: read from file '%s' failed\n",
+        avrdude_message(MSG_INFO, "%s: read from file %s failed\n",
-              progname, upd->filename);
+          progname, inname(upd->filename));
-      return -1;
+        return LIBAVRDUDE_GENERAL_FAILURE;
       }
-    v = avr_dup_part(p);
       size = rc;
+
+      if(memstats(p, upd->memtype, size, &fs) < 0)
+        return LIBAVRDUDE_GENERAL_FAILURE;
+    } else {
+      // Correct size of last read to include potentially cut off, trailing 0xff (flash)
+      size = fs.lastaddr+1;
+    }
+
+    v = avr_dup_part(p);
+
     if (quell_progress < 2) {
-      avrdude_message(MSG_NOTICE2, "%s: input file %s contains %d bytes\n",
+      if (userverify)
-            progname, upd->filename, size);
+        avrdude_message(MSG_NOTICE, "%s: input file %s contains %d byte%s\n",
-      avrdude_message(MSG_NOTICE2, "%s: reading on-chip %s%s data:\n",
+          progname, inname(upd->filename), fs.nbytes, plural(fs.nbytes));
+      avrdude_message(MSG_NOTICE2, "%s: reading on-chip %s%s data ...\n",
         progname, mem->desc, alias_mem_desc);
     }
 
     report_progress (0,1,"Reading");
     rc = avr_read(pgm, p, upd->memtype, v);
+    report_progress (1,1,NULL);
     if (rc < 0) {
       avrdude_message(MSG_INFO, "%s: failed to read all of %s%s memory, rc=%d\n",
         progname, mem->desc, alias_mem_desc, rc);
       pgm->err_led(pgm, ON);
       avr_free_part(v);
-      return -1;
+      return LIBAVRDUDE_GENERAL_FAILURE;
     }
-    report_progress (1,1,NULL);
 
-
+    if (quell_progress < 2)
-
-    if (quell_progress < 2) {
       avrdude_message(MSG_NOTICE2, "%s: verifying ...\n", progname);
-    }
+
     rc = avr_verify(p, v, upd->memtype, size);
     if (rc < 0) {
       avrdude_message(MSG_INFO, "%s: verification error; content mismatch\n",
         progname);
       pgm->err_led(pgm, ON);
       avr_free_part(v);
-      return -1;
+      return LIBAVRDUDE_GENERAL_FAILURE;
     }
 
     if (quell_progress < 2) {
-      avrdude_message(MSG_INFO, "%s: %d bytes of %s%s verified\n",
+      int verified = fs.nbytes+fs.ntrailing;
-              progname, rc, mem->desc, alias_mem_desc);
+      avrdude_message(MSG_INFO, "%s: %d byte%s of %s%s verified\n",
+        progname, verified, plural(verified), mem->desc, alias_mem_desc);
     }
 
     pgm->vfy_led(pgm, OFF);
     avr_free_part(v);
-  }
+    break;
-  else {
+
+  default:
     avrdude_message(MSG_INFO, "%s: invalid update operation (%d) requested\n",
       progname, upd->op);
-    return -1;
+    return LIBAVRDUDE_GENERAL_FAILURE;
   }
 
-  return 0;
+  return LIBAVRDUDE_SUCCESS;
 }
-