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 -c <wildcard>/dev_options for printing programmer entries of avrdude.conf #1059

Merged
merged 19 commits into from
Aug 14, 2022

Conversation

stefanrueger
Copy link
Collaborator

Try avrdude -c*/h and play with it.

Now avrdude -c* -p* should output a file that could be used to replace avrdude.conf for the part and programmer definitions.

Am aware that currently avrdude.conf has a lot of very useful comments, particularly the programmers (less so the parts). Am thinking about how to record comments during the lex/yacc parsing phase so that they can be printed at the right location during avrdude -c* -p*. But that's for another day.

Introduced -p <part>/A, which prints what -p <part>/S used to print, ie, all
components of the part structures including the default ones. Now -p <part>/S
prints the expanded part structure without use of parent and without printing
default values. This functionality is new and predominantly needed for
checking specific avrdude.conf entries, eg, avrdude -p*/St | grep pollindex

The option -p <part>/s continues to print a short entry of `avrdude.conf`
using its parent if so defined:

$ avrdude -p m328p/s

part parent "m328"
    desc                = "ATmega328P";
    id                  = "m328p";
    signature           = 0x1e 0x95 0x0f;
;
Also changed usbdev, usbsn, usbvendor and usbproduct components from
PROGRAMMER structure to be cached string pointers rather than fixed-size
arrays. These will be initialised by pgm_new() with a pointer to nul;
@MCUdude
Copy link
Collaborator

MCUdude commented Aug 8, 2022

I'm getting a compiler error on macOS:

pgm_type.c:132:13: error: conflicting types for 'locate_programmer_type_id'
const char *locate_programmer_type_id(void (*initpgm)(struct programmer_t *pgm)) {
            ^
./libavrdude.h:981:13: note: previous declaration is here
const char *locate_programmer_type_id(const void (*initpgm)(struct programmer_t *pgm));
            ^
1 error generated.
make[2]: *** [libavrdude_a-pgm_type.o] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2

After adding const to the first parameter in *locate_programmer_type_id, I'm getting the following warnings:

pgm_type.c:134:37: warning: comparison of distinct pointer types ('void (*)(struct programmer_t *)' and
      'const void (*)(struct programmer_t *)') [-Wcompare-distinct-pointer-types]
    if(programmers_types[i].initpgm == initpgm)
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~
1 warning generated.
  CC       libavrdude_a-pickit2.o
  CC       libavrdude_a-pindefs.o
pindefs.c:364:29: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int]
      p += sprintf(p, "~%d" + !(pindef->inverse[index] & (1 << bit)), pin);
                      ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/secure/_stdio.h:47:56: note: expanded from macro 'sprintf'
  __builtin___sprintf_chk (str, 0, __darwin_obsz(str), __VA_ARGS__)
                                                       ^~~~~~~~~~~
pindefs.c:364:29: note: use array indexing to silence this warning
      p += sprintf(p, "~%d" + !(pindef->inverse[index] & (1 << bit)), pin);
                            ^
                      &     [
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/secure/_stdio.h:47:56: note: expanded from macro 'sprintf'
  __builtin___sprintf_chk (str, 0, __darwin_obsz(str), __VA_ARGS__)
                                                       ^
1 warning generated.

@stefanrueger
Copy link
Collaborator Author

OK - I fixed the MacOS error and warning. I think. The errors we get now may be owing to a changed setup of how flex/bison is being provided for the CI checking. It worked a few minutes ago.

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 8, 2022

OK - I fixed the MacOS error and warning.

Thanks! It compiles fine on MacOS now, and it works as described in your first post. Great work!

@dl8dtl
Copy link
Contributor

dl8dtl commented Aug 8, 2022

Am thinking about how to record comments during the lex/yacc parsing phase

Maybe we should move the useful comments away from being throw-away stuff, and get them their own "comment = " grammar entry? Not to be used anywhere inside the normal source code, but that should make it easier to maintain them for conf file rewrites.

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 8, 2022

Maybe we should move the useful comments away from being throw-away stuff, and get them their own "comment = " grammar entry? Not to be used anywhere inside the normal source code, but that should make it easier to maintain them for conf file rewrites.

Sounds like a good idea to me!

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 8, 2022

Currently, there's two types of comments, the "header" on top, and comments between or after lines. Both of them usually provide useful information.

Maybe we should move the useful comments away from being throw-away stuff, and get them their own "comment = " grammar entry?

"comment = " makes sense for the header, but it is not clear to me how we should deal with inline comments in an elegant way while not impairing the overall readability.

# FTDI USB to serial cable TTL-232R-5V with a custom adapter for ICSP
# http://www.ftdichip.com/Products/Cables/USBTTLSerial.htm
# http://www.ftdichip.com/Support/Documents/DataSheets/Cables/DS_TTL-232R_CABLES.pdf
# For ICSP pinout see for example http://www.atmel.com/images/doc2562.pdf
# (Figure 1. ISP6PIN header pinout and Table 1. Connections required for ISP ...)
# TTL-232R GND 1 Black  -> ICPS GND   (pin 6)
# TTL-232R CTS 2 Brown  -> ICPS MOSI  (pin 4)
# TTL-232R VCC 3 Red    -> ICPS VCC   (pin 2)
# TTL-232R TXD 4 Orange -> ICPS RESET (pin 5)
# TTL-232R RXD 5 Yellow -> ICPS SCK   (pin 3)
# TTL-232R RTS 6 Green  -> ICPS MISO  (pin 1)
# Except for VCC and GND, you can connect arbitual pairs as long as 
# the following table is adjusted.
programmer
  id    = "ttl232r";
  desc  = "FTDI TTL232R-5V with ICSP adapter";
  type  = "ftdi_syncbb";
  connection_type = usb;
  miso  = 2; # rts
  sck   = 1; # rxd
  mosi  = 3; # cts
  reset = 0; # txd
;

@mcuee mcuee added the enhancement New feature or request label Aug 9, 2022
@mcuee
Copy link
Collaborator

mcuee commented Aug 9, 2022

Nor so sure why the CI build for MSVC failed -- probably a temporary download issue of winflexbison. I can build with VS2022 without issues (also MSYS2 mingw64).

@mcuee
Copy link
Collaborator

mcuee commented Aug 9, 2022

Now avrdude -c* -p* should output a file that could be used to replace avrdude.conf for the part and programmer definitions.

@stefanrueger
Somehow this doe not work properly.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059 -c* -p* >avrdude_pr1059_gen.conf

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059 -C .\avrdude_pr1059_gen.conf -c*/h
avrdude_pr1059.exe: error at C:\work\avr\avrdude_test\avrdude_bin\avrdude_pr1059_gen.conf:1: syntax error
avrdude_pr1059.exe: error reading system wide configuration file ".\avrdude_pr1059_gen.conf"

PS C:\work\avr\avrdude_test\avrdude_bin> head .\avrdude_pr1059_gen.conf
    desc                = "Wiring";
    type                = "wiring";
    connection_type     = serial;
    desc                = "Arduino";
    type                = "arduino";
    connection_type     = serial;
    desc                = "XBee Series 2 Over-The-Air (XBeeBoot)";
    type                = "xbee";
    connection_type     = serial;
    desc                = "FT2232D based generic programmer";

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059  -c*/h
avrdude_pr1059.exe: flags for developer option -c <wildcard>/<flags> not recognised
Wildcard examples (these need protecting in the shell through quoting):
         * all known programmers
   avrftdi just this programmer
  jtag*pdi matches jtag2pdi, jtag3pdi, jtag3updi and jtag2updi
  jtag?pdi matches jtag2pdi and jtag3pdi
Flags (one or more of the characters below):
       A  show entries of avrdude.conf programmers with all values
       S  show entries of avrdude.conf programmers with necessary values
       s  show short entries of avrdude.conf programmers using parent
       r  show entries of avrdude.conf programmers as raw dump
       t  use tab separated values as much as possible
Examples:
  $ avrdude -c usbasp/s
  $ avrdude -c */st | grep baudrate
  $ avrdude -c */r | sort
Notes:
  -c * is the same as -c */s
  This help message is printed using any unrecognised flag, eg, -c/h
  Leaving no space after -c can be an OK substitute for quoting in shells
  /s, /S and /A outputs are designed to be used as input in avrdude.conf
  Sorted /r output should stay invariant when rearranging avrdude.conf
  These options are just to help development, so not further documented

@dl8dtl
Copy link
Contributor

dl8dtl commented Aug 9, 2022

Nor so sure why the CI build for MSVC failed

I just re-triggered the run for it.

@stefanrueger
Copy link
Collaborator Author

@mcuee Clearly, the output on your system misses a lot of lines! Here is what I expect to happen:

$ avrdude "-p*"    "-c*"  >/tmp/avrdude.conf
$ avrdude "-p*/r" "-c*/r" >/tmp/avrdude.raw
$ avrdude "-p*/r" "-c*/r" -C /tmp/avrdude.conf | diff - /tmp/avrdude.raw
$ avrdude "-p*"    "-c*" | head
#------------------------------------------------------------
# wiring
#------------------------------------------------------------

programmer
    id                  = "wiring";
    desc                = "Wiring";
    type                = "wiring";
    connection_type     = serial;
;

The file of the generated syntax-correct output: /tmp/avrdude.conf

I have made a small change to the source that should not matter (redirection of stderr to stdout replaced by printing directly to stdout). Please could you try again and, if it still does not work, post the generated avrdude.conf file.

@stefanrueger
Copy link
Collaborator Author

Both of them usually provide useful information.

I fear you are right, @MCUdude. I am thinking along the lines of recording in lexer.l all # comments and the location relative to assignments (keyword = lines); then put that info into the programmer, part or memory structures as a linked list component LISTID comment during parsing inconfig_gram.y. Once done, all types of comments are there to (pretty much) print as they appeared in the source file. Only planning to capture # comments, not the multiline /* */ comments.

@mcuee
Copy link
Collaborator

mcuee commented Aug 9, 2022

I have made a small change to the source that should not matter (redirection of stderr to stdout replaced by printing directly to stdout). Please could you try again and, if it still does not work, post the generated avrdude.conf file.

@stefanrueger
This change is good, it fixed the issues under Windows.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059 -c* -p* >avrdude_pr1059_gen.conf
PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059v2.exe -c* -p* >.\avrdude_pr1059_gen1.conf

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059v2.exe -C .\avrdude_pr1059_gen.conf -cusbasp/s
avrdude_pr1059v2.exe: error at C:\work\avr\avrdude_test\avrdude_bin\avrdude_pr1059_gen.conf:1: syntax error
avrdude_pr1059v2.exe: error reading system wide configuration file ".\avrdude_pr1059_gen.conf"

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1059v2.exe -C .\avrdude_pr1059_gen1.conf -cusbasp/s
#------------------------------------------------------------
# usbasp
#------------------------------------------------------------

programmer
    id                  = "usbasp";
    desc                = "USBasp, http://www.fischl.de/usbasp/";
    type                = "usbasp";
    connection_type     = usb;
    usbvid              = 0x16c0;
    usbpid              = 0x05dc;
    usbvendor           = "www.fischl.de";
    usbproduct          = "USBasp";
;

MINGW64 /c/work/avr/avrdude_test/avrdude_sr/out/build/x64-Release/src
$ ./avrdude "-p*"    "-c*"  >/tmp/avrdude.conf
$ ./avrdude "-p*/r" "-c*/r" >/tmp/avrdude.raw
$ ./avrdude "-p*/r" "-c*/r" -C /tmp/avrdude.conf | diff - /tmp/avrdude.raw
$ ./avrdude "-p*"    "-c*" | head
#------------------------------------------------------------
# wiring
#------------------------------------------------------------

programmer
    id                  = "wiring";
    desc                = "Wiring";
    type                = "wiring";
    connection_type     = serial;
;

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 9, 2022

I am thinking along the lines of recording in lexer.l all # comments and the location relative to assignments (keyword = lines); then put that info into the programmer, part or memory structures as a linked list component LISTID comment during parsing inconfig_gram.y. Once done, all types of comments are there to (pretty much) print as they appeared in the source file. Only planning to capture # comments, not the multiline /* */ comments.

Sounds like a good idea!
The current format of avrdude.conf is all over the place when it comes to style and indentation, due to all the different patches and copy/pasting over the years.

Is the plan to -c*/s and -p*/s to create a more nicely formatted and more readable avrdude.conf that the parser would accept?

EDIT:

Now avrdude -c* -p* should output a file that could be used to replace avrdude.conf for the part and programmer definitions.

Apparently yes!

… otherwise

 - Replace strdup(s) with cfg_strdup(funname, s) that exits on out of mem
 - Replace malloc(n) with cfg_malloc(funname, n) that exits on out of mem
 - Change multiline string scanning in lexer.l to avoid core dump
 - Remove global variables string_buf and string_bug_ptr
 - Ensure reading strings unescapes strings C-Style
 - Ensure writing strings escapes strings C-Style again

Commit looks longer than needed as unescape() and auxiliary functions needed
to be moved from term.c (not in libavrdude) to config.c (in libavrdude).
This commit deals with default_programmer, default_serial, default_parallel
and default_spi. The long term objective is to remove all fixed-size buffers
from the structures that lexer.l and config_gram.y deal with.
This commit replaces fixed-string buffers in PROGRAMMER, AVRPART and AVRMEM
that are dealt with by the parser and grammar. Now, string assignments are
always to const char *, ie, these are read-only strings with arbitrary
length.

config_gram.y now only needs to consider one type of string assignment.

This commit also

  - Replaces the simple linear-search cache_string() function with faster
    hashed cache_string(). Either way, the returned value is likely to be
    shared, so should never be free()'d.

  - Duplicates hvupdi_support list in pgm_dup() and frees it in pgm_free()

  - Adds const qualifier to some function args in avrpart.c and pgm.c

  - Hardens some functions against being called with NULL pointers

  - Ensures _new() and _dup() functions for parts, programmers and memory
    return a suitable memory. Out of memory triggers exit in one of three
    functions, cfg_malloc(), cfg_realloc() and cfg_strdup(); there is
    rarely anything useful that AVRDUDE or, for that matter, any
    application compiled against libavrdude can do once you run out of
    memory as AVRDUDE/libavrdude rely heavily on allocation of memory.
@stefanrueger
Copy link
Collaborator Author

@MCUdude Yes, creating a syntax-correct avrdude.conf from the internal AVRDUDE representation after parsing is but one goal of the developer options. A more important goal is to be able (in a branch) to algorithmically rewrite some modified part descriptions (eg, using proper memory inheritance) to obtain a crisper and clearer part description. Another benefit should be that the raw (basically hexdump) output

avrdude  -p*/r -c*/r | sort >/tmp/avrdude.raw
# make some modifications in avrdude.conf to crisp it up
avrdude  -p*/r -c*/r | sort | diff - /tmp/avrdude.raw

could act as a proof that changes to avrdude.conf made no difference to the internal representation (so the new nicer avrdude.conf is equivalent to the older one). Who wants to proof-read 20k lines of configuration files?

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 10, 2022

@stefanrueger it would be great if we could re-structure avrdude.conf and implement "proper" inheritance to get the number of lines.

Maybe the megaAVR-0, tinyAVR-0,1,2, AVR-Dx and AVR-Ex can feature multiple inheritances? I'm not sure if this will make the conf file more challenging to read, but it will reduce the total number of lines.

For example: .avr_dx is the "parent". avr128da64 inherits .avr_dx, and avr128da48 inherits avr128da64.

PARENT			TARGET1			TARGET2
.avr_dx		<-	avr128da64	<-	avr128da48

@stefanrueger
Copy link
Collaborator Author

multiple inheritance

@MCUdude Yes, this is now possible, at the very least by hand. I was thinking that, algorithmically, there is the potential for a chip from a family to first inherit from the ".family" entry and then a potentially second time with a view to get the least lines/chars to describe the part. I wouldn't inherit more often to avoid that a user has to go back several inheritance levels. That could be done either once (in a quick and dirty branch) to generate an equivalent more compact avrdude.conf, or with an added option, say -p */s2 for print compact part desctiption allowing two inheritance levels. The most mileage would be for the classic parts.

@stefanrueger
Copy link
Collaborator Author

This commit finalises the PR and for the developer options. Now comments in avrdude.conf are associated to the

  • Space before programmer/part/memory definition
  • Line before an assignment
  • Right hand side of an assignment
  • Ending semicolon of a programmer/part/memory definition
$ avrdude -cttl232r/s
#------------------------------------------------------------
# ttl232r
#------------------------------------------------------------

# FTDI USB to serial cable TTL-232R-5V with a custom adapter for ICSP
# http://www.ftdichip.com/Products/Cables/USBTTLSerial.htm
# http://www.ftdichip.com/Support/Documents/DataSheets/Cables/DS_TTL-232R_CABLES.pdf
# For ICSP pinout see for example http://www.atmel.com/images/doc2562.pdf
# (Figure 1. ISP6PIN header pinout and Table 1. Connections required for ISP ...)
# TTL-232R GND 1 Black  -> ICPS GND   (pin 6)
# TTL-232R CTS 2 Brown  -> ICPS MOSI  (pin 4)
# TTL-232R VCC 3 Red    -> ICPS VCC   (pin 2)
# TTL-232R TXD 4 Orange -> ICPS RESET (pin 5)
# TTL-232R RXD 5 Yellow -> ICPS SCK   (pin 3)
# TTL-232R RTS 6 Green  -> ICPS MISO  (pin 1)
# Except for VCC and GND, you can connect arbitual pairs as long as 
# the following table is adjusted.
programmer
    id                  = "ttl232r";
    desc                = "FTDI TTL232R-5V with ICSP adapter";
    type                = "ftdi_syncbb";
    connection_type     = usb;
    reset               = 0; # txd
    sck                 = 1; # rxd
    mosi                = 3; # cts
    miso                = 2; # rts
;

This is as good as it gets (I reckon that's sufficient granularity). Remember, this is a developer option, with an intent to make our lives easier. Out of necessity the level of documentation and coverage of edge cases is less than for production user code.

Output lines always follow a fixed order, so if a comment refers to a block of assignments, the comment may move as it stays with the associated assignment. Example input:

#ISP-signals
    sck                 = 0; # AD0 (TCK)
    mosi                = 1; # AD1 (TDI)
    miso                = 2; # AD2 (TDO)
    reset               = 3; # AD3 (TMS)

Output:

    reset               = 3; # AD3 (TMS)
#ISP-signals
    sck                 = 0; # AD0 (TCK)
    mosi                = 1; # AD1 (TDI)
    miso                = 2; # AD2 (TDO)

The reason for this is that the comment was associated to the line before sck assignment. This needs correcting by hand (or can be ignored), but once corrected it stays.

@mcuee
Copy link
Collaborator

mcuee commented Aug 12, 2022

The reason for this is that the comment was associated to the line before sck assignment. This needs correcting by hand (or can be ignored), but once corrected it stays.

@stefanrueger

The generated avrdude.conf by avrdude -c* -p* looks pretty good now, including the comments. I will use it as the default conf file to see if there are any problems or not (no issues so far).

The situation for the reset before #ISP-signals only happen in one place. Maybe we can fix the current avrdude.conf first.

#------------------------------------------------------------
# ft232h
#------------------------------------------------------------

programmer
    id                  = "ft232h";
    desc                = "FT232H in MPSSE mode";
    type                = "avrftdi";
    connection_type     = usb;
    usbvid              = 0x0403;
    usbpid              = 0x6014;
    usbdev              = "A";
    reset               = 3; # AD3 (TMS)
#ISP-signals
    sck                 = 0; # AD0 (TCK)
    mosi                = 1; # AD1 (TDI)
    miso                = 2; # AD2 (TDO)
;

jtagkey mentioned is actually okay.

#------------------------------------------------------------
# jtagkey
#------------------------------------------------------------

programmer
    id                  = "jtagkey";
    desc                = "Amontec JTAGKey, JTAGKey-Tiny and JTAGKey2";
    type                = "avrftdi";
    connection_type     = usb;
    usbvid              = 0x0403;
# Note: This PID is used in all JTAGKey variants
    usbpid              = 0xcff8;
    usbdev              = "A";
    buff                = ~4;
#ISP-signals => 20 - Pin connector on JTAGKey
    reset               = 3; # TMS 7 violet
    sck                 = 0; # TCK 9 white
    mosi                = 1; # TDI 5 green
    miso                = 2; # TDO 13 orange
# VTG           VREF 1 brown with red tip
# GND           GND 20 black
# The colors are on the 20 pin breakout cable
# from Amontec
;

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 12, 2022

Thanks, @stefanrueger, it works like a charm! Some of the comments in avrdude.conf could need a cleanup or rewrite, but that's for another PR.

There are a few compiler warnings though:

developer_opts.c:405:9: warning: address of array 'd->descbuf' will always evaluate to 'true' [-Wpointer-bool-conversion]
  if(d->descbuf)
  ~~ ~~~^~~~~~~
developer_opts.c:408:9: warning: address of array 'd->idbuf' will always evaluate to 'true' [-Wpointer-bool-conversion]
  if(d->idbuf)
  ~~ ~~~^~~~~
developer_opts.c:411:9: warning: address of array 'd->family_idbuf' will always evaluate to 'true' [-Wpointer-bool-conversion]
  if(d->family_idbuf)
  ~~ ~~~^~~~~~~~~~~~

@mcuee
Copy link
Collaborator

mcuee commented Aug 12, 2022

MSYS2 mingw64 default build log to show the warnings (seems to be more stringent than Linux and macOS default build).

MINGW64 /c/work/avr/avrdude_test/avrdude_sr
$ ./build.sh
-- Building for: Ninja
-- The C compiler identification is GNU 12.1.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/cc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found Git: C:/msys64/usr/bin/git.exe (found version "2.37.1")
-- Found FLEX: C:/msys64/usr/bin/flex.exe (found version "2.6.4")
-- Found BISON: C:/msys64/usr/bin/bison.exe (found version "3.8.2")
-- Looking for libelf.h
-- Looking for libelf.h - found
-- Looking for libelf/libelf.h
-- Looking for libelf/libelf.h - found
-- Looking for usb.h
-- Looking for usb.h - not found
-- Looking for lusb0_usb.h
-- Looking for lusb0_usb.h - found
-- Looking for libusb.h
-- Looking for libusb.h - not found
-- Looking for libusb-1.0/libusb.h
-- Looking for libusb-1.0/libusb.h - found
-- Looking for hidapi/hidapi.h
-- Looking for hidapi/hidapi.h - found
-- Looking for ftdi_tcioflush
-- Looking for ftdi_tcioflush - found
-- Configuration summary:
-- ----------------------
-- DO HAVE    libelf
-- DO HAVE    libusb
-- DO HAVE    libusb_1_0
-- DO HAVE    libhidapi
-- DON'T HAVE libftdi
-- DO HAVE    libftdi1
-- DISABLED   doc
-- DISABLED   parport
-- DISABLED   linuxgpio
-- DISABLED   linuxspi
-- ----------------------
-- Configuring done
-- Generating done
-- Build files have been written to: C:/work/avr/avrdude_test/avrdude_sr/build_mingw64_nt-10.0-22000
[2/63] [BISON][Parser] Building parser with bison 3.8.2
config_gram.y: warning: 28 shift/reduce conflicts [-Wconflicts-sr]
config_gram.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
[41/63] Building C object src/CMakeFiles/libavrdude.dir/ser_win32.c.obj
C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c: In function 'net_send':
C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c:396:36: warning: pointer targets in passing argument 2 of 'send' differ in signedness [-Wpointer-sign]
  396 |                 rc = send(fd->ifd, p, (len > 1024) ? 1024 : len, 0);
      |                                    ^
      |                                    |
      |                                    const unsigned char *
In file included from C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c:30:
C:/msys64/mingw64/include/winsock2.h:1033:60: note: expected 'const char *' but argument is of type 'const unsigned char *'
 1033 |   WINSOCK_API_LINKAGE int WSAAPI send(SOCKET s,const char *buf,int len,int flags);
      |                                                ~~~~~~~~~~~~^~~
C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c: In function 'net_recv':
C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c:530:36: warning: pointer targets in passing argument 2 of 'recv' differ in signedness [-Wpointer-sign]
  530 |                 rc = recv(fd->ifd, p, (buflen - len > 1024) ? 1024 : buflen - len, 0);
      |                                    ^
      |                                    |
      |                                    unsigned char *
C:/msys64/mingw64/include/winsock2.h:1028:54: note: expected 'char *' but argument is of type 'unsigned char *'
 1028 |   WINSOCK_API_LINKAGE int WSAAPI recv(SOCKET s,char *buf,int len,int flags);
      |                                                ~~~~~~^~~
C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c: In function 'net_drain':
C:/work/avr/avrdude_test/avrdude_sr/src/ser_win32.c:696:36: warning: pointer targets in passing argument 2 of 'recv' differ in signedness [-Wpointer-sign]
  696 |                 rc = recv(fd->ifd, &buf, 1, 0);
      |                                    ^~~~
      |                                    |
      |                                    unsigned char *
C:/msys64/mingw64/include/winsock2.h:1028:54: note: expected 'char *' but argument is of type 'unsigned char *'
 1028 |   WINSOCK_API_LINKAGE int WSAAPI recv(SOCKET s,char *buf,int len,int flags);
      |                                                ~~~~~~^~~
[57/63] Building C object src/CMakeFiles/avrdude.dir/term.c.obj
In function 'tokenize',
    inlined from 'terminal_mode' at C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1301:12:
C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1188:15: warning: pointer 'buf' may be used after 'realloc' [-Wuse-after-free]
 1188 |       k = buf - bufp;
      |           ~~~~^~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1172:18: note: call to 'realloc' here
 1172 |       buf_tmp  = realloc(buf, bufsize);
      |                  ^~~~~~~~~~~~~~~~~~~~~
[60/63] Building C object src/CMakeFiles/libavrdude.dir/stk500v2.c.obj
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c: In function 'stk500v2_setparm':
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3104:6: warning: 'current_value' may be used uninitialized [-Wmaybe-uninitialized]
 3104 |   if (value == current_value) {
      |      ^
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3096:17: note: 'current_value' was declared here
 3096 |   unsigned char current_value;
      |                 ^~~~~~~~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c: In function 'stk500v2_print_parms1':
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3301:24: warning: 'osc_cmatch' may be used uninitialized [-Wmaybe-uninitialized]
 3301 |       f /= (osc_cmatch + 1);
      |            ~~~~~~~~~~~~^~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3248:47: note: 'osc_cmatch' was declared here
 3248 |   unsigned char vtarget, vadjust, osc_pscale, osc_cmatch, sck_duration =0; //XXX 0 is not correct, check caller
      |                                               ^~~~~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3248:35: warning: 'osc_pscale' may be used uninitialized [-Wmaybe-uninitialized]
 3248 |   unsigned char vtarget, vadjust, osc_pscale, osc_cmatch, sck_duration =0; //XXX 0 is not correct, check caller
      |                                   ^~~~~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3284:5: warning: 'vadjust' may be used uninitialized [-Wmaybe-uninitialized]
 3284 |     avrdude_message(MSG_INFO, "%sVaref           : %.1f V\n", p, vadjust / 10.0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3248:26: note: 'vadjust' was declared here
 3248 |   unsigned char vtarget, vadjust, osc_pscale, osc_cmatch, sck_duration =0; //XXX 0 is not correct, check caller
      |                          ^~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3273:5: warning: 'vtarget' may be used uninitialized [-Wmaybe-uninitialized]
 3273 |     avrdude_message(MSG_INFO, "%sVtarget         : %.1f V\n", p, vtarget / 10.0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/stk500v2.c:3248:17: note: 'vtarget' was declared here
 3248 |   unsigned char vtarget, vadjust, osc_pscale, osc_cmatch, sck_duration =0; //XXX 0 is not correct, check caller
      |                 ^~~~~~~
[62/63] Building C object src/CMakeFiles/avrdude.dir/developer_opts.c.obj
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c: In function 'avrpart_deep_copy':
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c:405:6: warning: the comparison will always evaluate as 'true' for the address of 'descbuf' will never be NULL [-Waddress]
  405 |   if(d->descbuf)
      |      ^
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c:382:8: note: 'descbuf' declared here
  382 |   char descbuf[64];
      |        ^~~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c:408:6: warning: the comparison will always evaluate as 'true' for the address of 'idbuf' will never be NULL [-Waddress]
  408 |   if(d->idbuf)
      |      ^
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c:383:8: note: 'idbuf' declared here
  383 |   char idbuf[32];
      |        ^~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c:411:6: warning: the comparison will always evaluate as 'true' for the address of 'family_idbuf' will never be NULL [-Waddress]
  411 |   if(d->family_idbuf)
      |      ^
C:/work/avr/avrdude_test/avrdude_sr/src/developer_opts.c:384:8: note: 'family_idbuf' declared here
  384 |   char family_idbuf[16];
      |        ^~~~~~~~~~~~
[63/63] Linking C executable src\avrdude.exe

Build succeeded.

Run

sudo cmake --build build_mingw64_nt-10.0-22000 --target install

to install.

@mcuee
Copy link
Collaborator

mcuee commented Aug 12, 2022

@stefanrueger

For compiler warnings, stk500v2.c and ser_win32.c are already covered in the following issue. So you may only need to check on the warnings for term.c and developer_opts.c.

@stefanrueger
Copy link
Collaborator Author

@mcuee Thanks! I removed the sources for the compiler warning bar one:

[57/63] Building C object src/CMakeFiles/avrdude.dir/term.c.obj
In function 'tokenize',
    inlined from 'terminal_mode' at C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1301:12:
C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1188:15: warning: pointer 'buf' may be used after 'realloc' [-Wuse-after-free]
 1188 |       k = buf - bufp;
      |           ~~~~^~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1172:18: note: call to 'realloc' here
 1172 |       buf_tmp  = realloc(buf, bufsize);
      |                  ^~~~~~~~~~~~~~~~~~~~~

This is a true warning that is a side effect of needing to adjust a possible change of memory location after a re-alloc, so in a sense this is needed unless one does a larger re-write to store indices relative to the base pointer as opposed to addresses.

@stefanrueger
Copy link
Collaborator Author

Should close Issue #964

@stefanrueger
Copy link
Collaborator Author

However, I was alarmed to see

config_gram.y: warning: 28 shift/reduce conflicts [-Wconflicts-sr]

in @mcuee's output (something that our cmake configuration hides). Just updated config_gram.y to remove the grammar ambiguity. The change that caused the shift/reduce conflicts wanted to make memory "abc"; with zero changes to the defaults a viable statement.

@mcuee
Copy link
Collaborator

mcuee commented Aug 12, 2022

@stefanrueger
Yes all the warnings are now gone except the one mentioned by you -- term.c.

 MINGW64 /c/work/avr/avrdude_test/avrdude_sr
$ ./build.sh
-- Building for: Ninja
-- The C compiler identification is GNU 12.1.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/cc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found Git: C:/msys64/usr/bin/git.exe (found version "2.37.1")
-- Found FLEX: C:/msys64/usr/bin/flex.exe (found version "2.6.4")
-- Found BISON: C:/msys64/usr/bin/bison.exe (found version "3.8.2")
-- Looking for libelf.h
-- Looking for libelf.h - found
-- Looking for libelf/libelf.h
-- Looking for libelf/libelf.h - found
-- Looking for usb.h
-- Looking for usb.h - not found
-- Looking for lusb0_usb.h
-- Looking for lusb0_usb.h - found
-- Looking for libusb.h
-- Looking for libusb.h - not found
-- Looking for libusb-1.0/libusb.h
-- Looking for libusb-1.0/libusb.h - found
-- Looking for hidapi/hidapi.h
-- Looking for hidapi/hidapi.h - found
-- Looking for ftdi_tcioflush
-- Looking for ftdi_tcioflush - found
-- Configuration summary:
-- ----------------------
-- DO HAVE    libelf
-- DO HAVE    libusb
-- DO HAVE    libusb_1_0
-- DO HAVE    libhidapi
-- DON'T HAVE libftdi
-- DO HAVE    libftdi1
-- DISABLED   doc
-- DISABLED   parport
-- DISABLED   linuxgpio
-- DISABLED   linuxspi
-- ----------------------
-- Configuring done
-- Generating done
-- Build files have been written to: C:/work/avr/avrdude_test/avrdude_sr/build_mingw64_nt-10.0-22000
[57/63] Building C object src/CMakeFiles/avrdude.dir/term.c.obj
In function 'tokenize',
    inlined from 'terminal_mode' at C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1301:12:
C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1188:15: warning: pointer 'buf' may 
be used after 'realloc' [-Wuse-after-free]
 1188 |       k = buf - bufp;
      |           ~~~~^~~~~~
C:/work/avr/avrdude_test/avrdude_sr/src/term.c:1172:18: note: call to 'realloc' here
 1172 |       buf_tmp  = realloc(buf, bufsize);
      |                  ^~~~~~~~~~~~~~~~~~~~~
[63/63] Linking C executable src\avrdude.exe

Build succeeded.

Run

sudo cmake --build build_mingw64_nt-10.0-22000 --target install

to install.

@mcuee mcuee mentioned this pull request Aug 12, 2022
@mcuee
Copy link
Collaborator

mcuee commented Aug 13, 2022

@stefanrueger
One more warning for macOS. It was mentioned in #964 as well.

/Users/mcuee/build/avr/avrdude/src/jtagmkII.c:491:18: warning: 
variable 'checksum' set but not used [-Wunused-but-set-variable]
  unsigned short checksum = 0;
                 ^
1 warning generated.

Edit: sorry, that is for git main. Your branch is okay.

@stefanrueger
Copy link
Collaborator Author

@MCUdude I have piggy-backed onto PR #1059 code so that specifying the full memory name now always works (even if a memory with longer name and same initial part exists).

@stefanrueger
Copy link
Collaborator Author

Likely to merge PR #1056, #1059 and #1063 during the weekend as they all have been looked at.

@mcuee
Copy link
Collaborator

mcuee commented Aug 14, 2022

Likely to merge PR #1056, #1059 and #1063 during the weekend as they all have been looked at.

Yes they are good to go based on the positive testing results.

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 14, 2022

@MCUdude I have piggy-backed onto PR #1059 code so that specifying the full memory name now always works (even if a memory with longer name and same initial part exists).

Excellent, thank you!

@stefanrueger stefanrueger merged commit 0b94ffd into avrdudes:main Aug 14, 2022
@stefanrueger stefanrueger deleted the programmer-devopts branch August 15, 2022 11:06
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.

4 participants