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

Harden AVRDUDE against pre-C99 libraries #1608

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

stefanrueger
Copy link
Collaborator

Apparently, some Windows 7 systems have C libraries that are not C99 compliant. This PR hardens against these problems.

@mcuee mcuee added the enhancement New feature or request label Jan 7, 2024
@mcuee
Copy link
Collaborator

mcuee commented Jan 7, 2024

@stefanrueger

Looks good except it generates new warnings from MSYS2 mingw compiler.

From github action MSYS2 mingw64 build
https://github.com/avrdudes/avrdude/actions/runs/7437554531/job/20235338846

In file included from D:/a/avrdude/avrdude/src/serbb_win32.c:26:
D:/a/avrdude/avrdude/src/serbb_win32.c: In function 'serbb_open':
D:/a/avrdude/avrdude/src/serbb_win32.c:302:77: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  302 |         pmsg_debug("ser_open(): opened comm port %s, handle 0x%lx\n", port, (unsigned long) hComPort);
      |                                                                             ^
D:/a/avrdude/avrdude/src/avrdude.h:81:121: note: in definition of macro 'pmsg_debug'
   81 | #define pmsg_debug(...)     avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_DEBUG, __VA_ARGS__)
      |                                                                                                                         ^~~~~~~~~~~
D:/a/avrdude/avrdude/src/serbb_win32.c: In function 'serbb_close':
D:/a/avrdude/avrdude/src/serbb_win32.c:318:68: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  318 |         pmsg_debug("ser_close(): closed comm port handle 0x%lx\n", (unsigned long) hComPort);
      |                                                                    ^
D:/a/avrdude/avrdude/src/avrdude.h:81:121: note: in definition of macro 'pmsg_debug'
   81 | #define pmsg_debug(...)     avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_DEBUG, __VA_ARGS__)
      |  

You do not see such warnings with git main. Please refer to the github action build log with git main.
https://github.com/avrdudes/avrdude/actions/runs/7397687783/job/20125270526

@mcuee
Copy link
Collaborator

mcuee commented Jan 7, 2024

@stefanrueger

BTW, once you merge this into git main, then the Arduino avrdude-packing script in avrdude repo will run. As of now the script does not run on pull-requests and only upon push to git main.

https://github.com/avrdudes/avrdude/blob/main/.github/workflows/arduino_packing.yml

name: avrdude_packing

env:
  # The name of the project
  PROJECT_NAME: avrdude
  DIST_DIR: dist
  ARTIFACT_NAME: dist

on:
  label:
    types:
      - created
  push:
    branches:
      - main

@mcuee
Copy link
Collaborator

mcuee commented Jan 7, 2024

Another thing, it seems that you are correct that Windows 7 C runtime is not really compatible with C99. But then I do not understand why MinGW MSYS2 build does not have the issue with Hex file type automatic recognition, since it also uses msvcrt.dll under Windows 7. It is said that Windows 10 comes with the universal C Runtime (UCRT) which is C99.
https://www.reddit.com/r/C_Programming/comments/4osr95/alternative_to_msvcrtdll/

However, our default MSYS2 build environment are mingw32 and mingw64 and not ucrt64.
https://www.msys2.org/docs/environments/

Name Prefix Toolchain Architecture C Library C++ Library
MSYS /usr gcc x86_64 cygwin libstdc++
UCRT64 /ucrt64 gcc x86_64 ucrt libstdc++
CLANG64 /clang64 llvm x86_64 ucrt libc++
CLANGARM64 /clangarm64 llvm aarch64 ucrt libc++
CLANG32 /clang32 llvm i686 ucrt libc++
MINGW64 /mingw64 gcc x86_64 msvcrt libstdc++
MINGW32 /mingw32 gcc i686 msvcrt libstdc++

@stefanrueger
Copy link
Collaborator Author

MinGW MSYS2 build does not have the issue with Hex file type automatic recognition

Maybe one is statically linked and one dynamically. The problem is the scanf() library function at execution.

@mcuee
Copy link
Collaborator

mcuee commented Jan 8, 2024

@stefanrueger

Now this PR is good to go. Please merge this so that we can confirm whether the :a issue is sorted out or not with this PR.

@mcuee
Copy link
Collaborator

mcuee commented Jan 8, 2024

However, our default MSYS2 build environment are mingw32 and mingw64 and not ucrt64. https://www.msys2.org/docs/environments/

I have created a new PR to add MSYS2 ucrt64, clang32 and clang64 build.

clang compilers sometimes capture more codes issues compared to gcc and MSVC.

@stefanrueger stefanrueger merged commit 2d872af into avrdudes:main Jan 8, 2024
10 checks passed
@stefanrueger stefanrueger deleted the z-scanf-modifier branch January 8, 2024 09:25
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.

avrdude git main issue with SNAP and PICKit 4 under Windows 7 -- MinGW build
2 participants