Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a dry run for -U updates before opening the programmer #1056

Merged
merged 4 commits into from
Aug 14, 2022

Conversation

stefanrueger
Copy link
Collaborator

This PR 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.

Whilst the update_dryrun() goes through some of the checks of do_op(), the functionality of do_op() has not been changed, so applications that use do_op() from libavrdude will work the same.

Needs thorough testing for cases where the dry run exits when, without it, the command would legitimately work. It is possible to construct cases where the dry run misses opportunities to check, but this is not an issue as this will be done during the real work.

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

Examples:

$ avrdude -U - -p m328p -c usb-bub-ii
avrdude: can't auto detect file format for stdin/out, specify explicitly

$ rm -f testin; touch testin
$ avrdude -U testin -p m328p -c usb-bub-ii
avrdude: can't determine file format for testin, specify explicitly

$ avrdude -qqU efuse:r:testin:i -U eeprom:w:testin:a -p m328p -c usb-bub-ii && echo OK
OK

$ avrdude -qqU efuse:r:testin:i -U eeprom:w:festin:a -p m328p -c usb-bub-ii && echo OK
avrdude: file festin is not readable. No such file or directory

$ chmod -r testin 
$ avrdude -U testin -p m328p -c usb-bub-ii
avrdude: file testin is not readable. Permission denied

$ avrdude -U flash:r:testin:r -U another -p m328p -c usb-bub-ii
avrdude: file another is not readable. No such file or directory

$ avrdude -U - -U typo:r:.:h -U eeprom:w:testin:r -p m328p -c usb-bub-ii
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. Permission denied

$ rm -f testin

$ avrdude -U efuse:w:0xff:m -qqp ATmega8 -c usb-bub-ii && echo OK
avrdude: skipping -U efuse:... as memory not defined for part ATmega8
OK

$ avrdude -U efuse:r:0xff:m -qqp ATmega8 -c usb-bub-ii && echo OK
avrdude: invalid file format 'immediate' for output

$ avrdude -U eeprom:w:/dev/urandom:r -qqp m328p -c usb-bub-ii && echo OK
OK

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
@stefanrueger stefanrueger added the enhancement New feature or request label Aug 5, 2022
@MCUdude MCUdude modified the milestone: AVRDUDE 7.1 Aug 5, 2022
@MCUdude
Copy link
Collaborator

MCUdude commented Aug 5, 2022

I haven't thought about this potential issue before.

Without this PR, if I specify that Avrdude can't read from, it still automatically erases the flash memory, which IMO is not the "principle of least surprise".

(Without this PR):

$ cat test-error.hex 
:020000040000FA
:20000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00
:20002000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0
:20008000000000000000000000000000000000000000000000000000000000000000000060
:2000A000000000000000000000000000000000000000000000000000000000000000000040
:2000C000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF40
:2000E000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF20
:20010000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
:00000001FF

$ chmod -r test-error.hex

$ ./avrdude -patmega1284p -cusbasp -Uflash:w:test-error.hex:i

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e9705 (probably m1284p)
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: can't open input file test-error.hex: Permission denied
avrdude: reading input file test-error.hex for flash
avrdude: read from file test-error.hex failed

avrdude done.  Thank you.

With PR:

$ ./avrdude -patmega1284p -cusbasp -Uflash:w:test-error.hex:i
avrdude: file test-error.hex is not readable. Permission denied

@mcuee
Copy link
Collaborator

mcuee commented Aug 6, 2022

I think this PR is a good one. Tested a few cases and all seem to be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants