From b24a1cf6673923a9db502742308d925ab53994fc Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Fri, 5 Aug 2022 17:38:59 +0100 Subject: [PATCH 1/4] Implement a dry run for -U updates before opening the programmer This commit checks -U update requests for - Typos in memory names - Whether the files can be written or read - Automatic format detection if necessary before opening the programmer. This to reduce the chances of the programming failing midway through. Minor additional changes: - Give strerror() system info when files are not read/writeable - Lift the auto detection message from MSG_INFO to MSG_NOTICE - Provide fileio_fmt_autodetect() in the AVRDUDE library - Rename fmtstr() in the AVRDUDE library to fileio_fmtstr() to avoid name clashes when an application links with it Example: $ avrdude -U - -U typo:r:.:h -U eeprom:w:testin:r -p ... -c ... avrdude: can't auto detect file format for stdin/out, specify explicitly avrdude: unknown memory type typo avrdude: file . is not writeable (not a regular or character file?) avrdude: file testin is not readable. No such file or directory --- src/fileio.c | 13 ++-- src/libavrdude.h | 12 +++- src/main.c | 20 ++++--- src/update.c | 152 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 175 insertions(+), 22 deletions(-) diff --git a/src/fileio.c b/src/fileio.c index d21d8993..9d39325a 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -100,11 +100,8 @@ static int fileio_num(struct fioparms * fio, char * filename, FILE * f, AVRMEM * mem, int size, FILEFMT fmt); -static int fmt_autodetect(char * fname); - - -char * fmtstr(FILEFMT format) +char * fileio_fmtstr(FILEFMT format) { switch (format) { case FMT_AUTO : return "auto-detect"; break; @@ -1402,7 +1399,7 @@ int fileio_setparms(int op, struct fioparms * fp, -static int fmt_autodetect(char * fname) +int fileio_fmt_autodetect(const char * fname) { FILE * f; unsigned char buf[MAX_LINE_LEN]; @@ -1547,7 +1544,7 @@ int fileio(int oprwv, char * filename, FILEFMT format, return -1; } - format_detect = fmt_autodetect(fname); + format_detect = fileio_fmt_autodetect(fname); if (format_detect < 0) { avrdude_message(MSG_INFO, "%s: can't determine file format for %s, specify explicitly\n", progname, fname); @@ -1556,8 +1553,8 @@ int fileio(int oprwv, char * filename, FILEFMT format, format = format_detect; if (quell_progress < 2) { - avrdude_message(MSG_INFO, "%s: %s file %s auto detected as %s\n", - progname, fio.iodesc, fname, fmtstr(format)); + avrdude_message(MSG_NOTICE, "%s: %s file %s auto detected as %s\n", + progname, fio.iodesc, fname, fileio_fmtstr(format)); } } diff --git a/src/libavrdude.h b/src/libavrdude.h index 23905953..5e8fc5d2 100644 --- a/src/libavrdude.h +++ b/src/libavrdude.h @@ -872,7 +872,9 @@ enum { extern "C" { #endif -char * fmtstr(FILEFMT format); +char * fileio_fmtstr(FILEFMT format); + +int fileio_fmt_autodetect(const char * fname); int fileio(int oprwv, char * filename, FILEFMT format, struct avrpart * p, char * memtype, int size); @@ -936,6 +938,14 @@ const char *update_inname(const char *fn); const char *update_outname(const char *fn); const char *update_interval(int a, int b); +// Helper functions for dry run to determine file access +int update_is_okfile(const char *fn); +int update_is_writeable(const char *fn); +int update_is_readable(const char *fn); + +int update_dryrun(struct avrpart *p, UPDATE *upd); + + #ifdef __cplusplus } #endif diff --git a/src/main.c b/src/main.c index 9f72847f..de991aab 100644 --- a/src/main.c +++ b/src/main.c @@ -901,11 +901,13 @@ int main(int argc, char * argv []) } /* - * Now that we know which part we are going to program, locate any - * -U options using the default memory region, and fill in the - * device-dependent default region name, either "application" (for - * Xmega devices), or "flash" (everything else). + * Now that we know which part we are going to program, locate any -U + * options using the default memory region, fill in the device-dependent + * default region name ("application" for Xmega parts or "flash" otherwise) + * and check for basic problems with memory names or file access with a + * view to exit before programming. */ + int doexit = 0; for (ln=lfirst(updates); ln; ln=lnext(ln)) { upd = ldata(ln); if (upd->memtype == NULL) { @@ -920,12 +922,12 @@ int main(int argc, char * argv []) } } - if (!avr_mem_might_be_known(upd->memtype)) { - avrdude_message(MSG_INFO, "%s: unknown memory type %s\n", progname, upd->memtype); - exit(1); - } - // TODO: check whether filename other than "-" is readable/writable + rc = update_dryrun(p, upd); + if (rc && rc != LIBAVRDUDE_SOFTFAIL) + doexit = 1; } + if(doexit) + exit(1); /* * open the programmer diff --git a/src/update.c b/src/update.c index 2aa2579b..b0c48c6d 100644 --- a/src/update.c +++ b/src/update.c @@ -24,6 +24,9 @@ #include #include #include +#include +#include +#include #include "ac_cfg.h" #include "avrdude.h" @@ -319,6 +322,147 @@ const char *update_interval(int a, int b) { } +// Helper functions for dry run to determine file access + +int update_is_okfile(const char *fn) { + struct stat info; + + // File exists and is a regular file or a character file, eg, /dev/urandom + return fn && *fn && stat(fn, &info) == 0 && !!(info.st_mode & (S_IFREG | S_IFCHR)); +} + +int update_is_writeable(const char *fn) { + if(!fn || !*fn) + return 0; + + // Assume writing to stdout will be OK + if(!strcmp(fn, "-")) + return 1; + + // File exists? If so return whether it's readable and an OK file type + if(access(fn, F_OK) == 0) + return access(fn, W_OK) == 0 && update_is_okfile(fn); + + // File does not exist: try to create it + FILE *test = fopen(fn, "w"); + if(test) { + unlink(fn); + fclose(test); + } + return !!test; +} + +int update_is_readable(const char *fn) { + if(!fn || !*fn) + return 0; + + // Assume reading from stdin will be OK + if(!strcmp(fn, "-")) + return 1; + + // File exists, is readable by the process and an OK file type? + return access(fn, R_OK) == 0 && update_is_okfile(fn); +} + + +static void ioerror(const char *iotype, UPDATE *upd) { + avrdude_message(MSG_INFO, "%s: file %s is not %s", + progname, update_outname(upd->filename), iotype); + if(errno) { + char buf[1024]; + strerror_r(errno, buf, sizeof buf); + avrdude_message(MSG_INFO, ". %s", buf); + } else if(upd->filename && *upd->filename) + avrdude_message(MSG_INFO, " (not a regular or character file?)"); + avrdude_message(MSG_INFO, "\n"); +} + +// Basic checks to reveal serious failure before programming +int update_dryrun(struct avrpart *p, UPDATE *upd) { + static char **wrote; + static int nfwritten; + + int known, format_detect, ret = LIBAVRDUDE_SUCCESS; + + /* + * Reject an update if memory name is not known amongst any part (suspect a typo) + * but accept when the specific part does not have it (allow unifying i/faces) + */ + if(!avr_mem_might_be_known(upd->memtype)) { + avrdude_message(MSG_INFO, "%s: unknown memory type %s\n", progname, upd->memtype); + ret = LIBAVRDUDE_GENERAL_FAILURE; + } else if(p && !avr_locate_mem(p, upd->memtype)) + ret = LIBAVRDUDE_SOFTFAIL; + + known = 0; + // Necessary to check whether the file is readable? + if(upd->op == DEVICE_VERIFY || upd->op == DEVICE_WRITE || upd->format == FMT_AUTO) { + if(upd->format != FMT_IMM) { + // Need to read the file: was it written before, so will be known? + for(int i = 0; i < nfwritten; i++) + if(!wrote || (upd->filename && !strcmp(wrote[i], upd->filename))) + known = 1; + + errno = 0; + if(!known && !update_is_readable(upd->filename)) { + ioerror("readable", upd); + ret = LIBAVRDUDE_GENERAL_FAILURE; + known = 1; // Pretend we know it, so no auto detect needed + } + } + } + + if(!known && upd->format == FMT_AUTO) { + if(!strcmp(upd->filename, "-")) { + avrdude_message(MSG_INFO, "%s: can't auto detect file format for stdin/out, " + "specify explicitly\n", progname); + ret = LIBAVRDUDE_GENERAL_FAILURE; + } else if((format_detect = fileio_fmt_autodetect(upd->filename)) < 0) { + avrdude_message(MSG_INFO, "%s: can't determine file format for %s, specify explicitly\n", + progname, upd->filename); + ret = LIBAVRDUDE_GENERAL_FAILURE; + } else { + // Set format now, no need to repeat auto detection later + upd->format = format_detect; + if(quell_progress < 2) + avrdude_message(MSG_NOTICE, "%s: %s file %s auto detected as %s\n", + progname, upd->op == DEVICE_READ? "output": "input", upd->filename, + fileio_fmtstr(upd->format)); + } + } + + switch(upd->op) { + case DEVICE_READ: + if(upd->format == FMT_IMM) { + avrdude_message(MSG_INFO, + "%s: invalid file format 'immediate' for output\n", progname); + ret = LIBAVRDUDE_GENERAL_FAILURE; + } else { + errno = 0; + if(!update_is_writeable(upd->filename)) { + ioerror("writeable", upd); + ret = LIBAVRDUDE_GENERAL_FAILURE; + } else if(upd->filename) { // Record filename (other than stdout) is available for future reads + if(strcmp(upd->filename, "-") && (wrote = realloc(wrote, sizeof(*wrote) * (nfwritten+1)))) + wrote[nfwritten++] = upd->filename; + } + } + break; + + case DEVICE_VERIFY: // Already checked that file is readable + case DEVICE_WRITE: + break; + + default: + avrdude_message(MSG_INFO, "%s: invalid update operation (%d) requested\n", + progname, upd->op); + ret = LIBAVRDUDE_GENERAL_FAILURE; + } + + return ret; +} + + int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags flags) { struct avrpart * v; @@ -346,7 +490,7 @@ int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags f // 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", progname); + "%s: invalid file format 'immediate' for output\n", progname); return LIBAVRDUDE_GENERAL_FAILURE; } if (quell_progress < 2) @@ -383,15 +527,15 @@ int do_op(PROGRAMMER * pgm, struct avrpart * p, UPDATE * upd, enum updateflags f // 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, update_inname(upd->filename), mem->desc, alias_mem_desc); if (rc < 0) { avrdude_message(MSG_INFO, "%s: read from file %s failed\n", progname, update_inname(upd->filename)); return LIBAVRDUDE_GENERAL_FAILURE; } size = rc; + if (quell_progress < 2) + avrdude_message(MSG_INFO, "%s: reading input file %s for %s%s\n", + progname, update_inname(upd->filename), mem->desc, alias_mem_desc); if(memstats(p, upd->memtype, size, &fs) < 0) return LIBAVRDUDE_GENERAL_FAILURE; From e590aead9323a3ad72c33b0d0ca6823cc023faec Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Fri, 5 Aug 2022 18:04:46 +0100 Subject: [PATCH 2/4] Treat comparison of different signedness warning in fileio.c --- src/fileio.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/fileio.c b/src/fileio.c index 9d39325a..52e9afef 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -346,7 +346,7 @@ static int ihex2b(char * infile, FILE * inf, return -1; } nextaddr = ihex.loadofs + baseaddr - fileoffset; - if (nextaddr + ihex.reclen > bufsize) { + if (nextaddr + ihex.reclen > (unsigned) bufsize) { avrdude_message(MSG_INFO, "%s: ERROR: address 0x%04x out of range at line %d of %s\n", progname, nextaddr+ihex.reclen, lineno, infile); return -1; @@ -412,7 +412,6 @@ static int b2srec(unsigned char * inbuf, int bufsize, unsigned char * buf; unsigned int nextaddr; int n, nbytes, addr_width; - int i; unsigned char cksum; char * tmpl=0; @@ -460,10 +459,10 @@ static int b2srec(unsigned char * inbuf, int bufsize, cksum += n + addr_width + 1; - for (i=addr_width; i>0; i--) + for (int i=addr_width; i>0; i--) cksum += (nextaddr >> (i-1) * 8) & 0xff; - for (i=nextaddr; i0; i--) + for (int i=addr_width; i>0; i--) cksum += (nextaddr >> (i - 1) * 8) & 0xff; cksum = 0xff - cksum; fprintf(outf, "%02X\n", cksum); @@ -597,7 +596,7 @@ static int srec2b(char * infile, FILE * inf, int len; struct ihexrec srec; int rc; - int reccount; + unsigned int reccount; unsigned char datarec; char * msg = 0; @@ -687,7 +686,7 @@ static int srec2b(char * infile, FILE * inf, return -1; } nextaddr -= fileoffset; - if (nextaddr + srec.reclen > bufsize) { + if (nextaddr + srec.reclen > (unsigned) bufsize) { avrdude_message(MSG_INFO, msg, progname, nextaddr+srec.reclen, "", lineno, infile); return -1; @@ -1006,8 +1005,7 @@ static int elf2b(char * infile, FILE * inf, * ELF file region for these, and extract the actual byte to write * from it, using the "foff" offset obtained above. */ - if (mem->size != 1 && - sh->sh_size > mem->size) { + if (mem->size != 1 && sh->sh_size > (unsigned) mem->size) { avrdude_message(MSG_INFO, "%s: ERROR: section \"%s\" does not fit into \"%s\" memory:\n" " 0x%x + %u > %u\n", progname, sname, mem->desc, @@ -1507,7 +1505,7 @@ int fileio(int oprwv, char * filename, FILEFMT format, if (rc < 0) return -1; - if (fio.op == FIO_READ) + if (size < 0 || fio.op == FIO_READ) size = mem->size; if (fio.op == FIO_READ) { From e681035cc45a9cd6341736de7f77d526e6a37915 Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Fri, 5 Aug 2022 18:47:40 +0100 Subject: [PATCH 3/4] Add strerror_r() and access() modes to MSVC compat file --- src/msvc/msvc_compat.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/msvc/msvc_compat.h b/src/msvc/msvc_compat.h index eb3e026a..2c072b3b 100644 --- a/src/msvc/msvc_compat.h +++ b/src/msvc/msvc_compat.h @@ -28,6 +28,11 @@ #pragma comment(lib, "ws2_32.lib") #pragma comment(lib, "setupapi.lib") +#define strerror_r(errno,buf,len) strerror_s(buf,len,errno) + +#define R_OK 4 +#define W_OK 2 +#define X_OK 1 #define F_OK 0 #define PATH_MAX _MAX_PATH From a8bbedcde399492d9dccd1c15ea5d1c25ab798af Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Sat, 6 Aug 2022 00:20:43 +0100 Subject: [PATCH 4/4] Switch from strerror_r() to strerror() in update.c for portability --- src/update.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/update.c b/src/update.c index b0c48c6d..b79f6442 100644 --- a/src/update.c +++ b/src/update.c @@ -368,13 +368,11 @@ int update_is_readable(const char *fn) { static void ioerror(const char *iotype, UPDATE *upd) { avrdude_message(MSG_INFO, "%s: file %s is not %s", progname, update_outname(upd->filename), iotype); - if(errno) { - char buf[1024]; - strerror_r(errno, buf, sizeof buf); - avrdude_message(MSG_INFO, ". %s", buf); - } else if(upd->filename && *upd->filename) + if(errno) + avrdude_message(MSG_INFO, ". %s", strerror(errno)); + else if(upd->filename && *upd->filename) avrdude_message(MSG_INFO, " (not a regular or character file?)"); - avrdude_message(MSG_INFO, "\n"); + avrdude_message(MSG_INFO, "\n"); } // Basic checks to reveal serious failure before programming