flashrom

One more question, is how to test WP functionality on MX25L1605E

Copying output from the link:

mm@DESKTOP-4AITTRD /z/Projektek/flashrom_git
$ meson compile -C builddir
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: "c:\Program Files\Meson\ninja.EXE" -C //VBoxSvr/mm/Projektek/flashrom_git/builddir
ninja: Entering directory `//VBoxSvr/mm/Projektek/flashrom_git/builddir'
[7/116] Compiling C object libflashrom-1.dll.p/helpers_fileio.c.obj
FAILED: libflashrom-1.dll.p/helpers_fileio.c.obj
"gcc" "-Ilibflashrom-1.dll.p" "-I." "-I.." "-I..\include" "-IC:\Program Files (x86)\National Instruments\Ni-845x\MS Visual C" "-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-Wextra" "-Werror" "-std=c99" "-Os" "-Wshadow" "-Wmissing-prototypes" "-Wwrite-strings" "-Wno-unused-parameter" "-Wno-missing-braces" "-D_DEFAULT_SOURCE" "-D_POSIX_C_SOURCE=200809L" "-D_BSD_SOURCE" "-D__BSD_VISIBLE" "-D__XSI_VISIBLE" "-D_NETBSD_SOURCE" "-D_DARWIN_C_SOURCE" "-DFLASHROM_VERSION=\"1.4.0-devel (git:v1.2-1336-gd666a818)\"" "-DHAVE_STRNLEN=1" "-DIS_WINDOWS=1" "-D__FLASHROM_LITTLE_ENDIAN__=1" "-DCONFIG_NI845X_SPI=1" "-DCONFIG_DEFAULT_PROGRAMMER_NAME=NULL" "-DCONFIG_DEFAULT_PROGRAMMER_ARGS=\"\"" -MD -MQ libflashrom-1.dll.p/helpers_fileio.c.obj -MF "libflashrom-1.dll.p\helpers_fileio.c.obj.d" -o libflashrom-1.dll.p/helpers_fileio.c.obj "-c" ../helpers_fileio.c
../helpers_fileio.c: In function 'read_buf_from_file':
../helpers_fileio.c:41:11: error: implicit declaration of function 'fdopen' [-Werror=implicit-function-declaration]
   image = fdopen(fileno(stdin), "rb");
           ^~~~~~
../helpers_fileio.c:41:18: error: implicit declaration of function 'fileno' [-Werror=implicit-function-declaration]
   image = fdopen(fileno(stdin), "rb");
                  ^~~~~~
../helpers_fileio.c:41:9: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   image = fdopen(fileno(stdin), "rb");
         ^
cc1.exe: all warnings being treated as errors
[12/116] Compiling C object libflashrom-1.dll.p/flashrom.c.obj
FAILED: libflashrom-1.dll.p/flashrom.c.obj
"gcc" "-Ilibflashrom-1.dll.p" "-I." "-I.." "-I..\include" "-IC:\Program Files (x86)\National Instruments\Ni-845x\MS Visual C" "-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-Wextra" "-Werror" "-std=c99" "-Os" "-Wshadow" "-Wmissing-prototypes" "-Wwrite-strings" "-Wno-unused-parameter" "-Wno-missing-braces" "-D_DEFAULT_SOURCE" "-D_POSIX_C_SOURCE=200809L" "-D_BSD_SOURCE" "-D__BSD_VISIBLE" "-D__XSI_VISIBLE" "-D_NETBSD_SOURCE" "-D_DARWIN_C_SOURCE" "-DFLASHROM_VERSION=\"1.4.0-devel (git:v1.2-1336-gd666a818)\"" "-DHAVE_STRNLEN=1" "-DIS_WINDOWS=1" "-D__FLASHROM_LITTLE_ENDIAN__=1" "-DCONFIG_NI845X_SPI=1" "-DCONFIG_DEFAULT_PROGRAMMER_NAME=NULL" "-DCONFIG_DEFAULT_PROGRAMMER_ARGS=\"\"" -MD -MQ libflashrom-1.dll.p/flashrom.c.obj -MF "libflashrom-1.dll.p\flashrom.c.obj.d" -o libflashrom-1.dll.p/flashrom.c.obj "-c" ../flashrom.c
../flashrom.c: In function 'programmer_init':
../flashrom.c:162:16: error: implicit declaration of function 'strdup' [-Werror=implicit-function-declaration]
   cfg.params = strdup(param);
                ^~~~~~
../flashrom.c:162:14: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   cfg.params = strdup(param);
              ^
../flashrom.c: In function 'get_flash_region':
../flashrom.c:380:16: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   region->name = strdup("");
                ^
../flashrom.c: In function 'flashbuses_to_text':
../flashrom.c:939:10: error: return makes pointer from integer without a cast [-Werror=int-conversion]
   return strdup("Non-SPI");
          ^~~~~~~~~~~~~~~~~
../flashrom.c:941:10: error: return makes pointer from integer without a cast [-Werror=int-conversion]
   return strdup("None");
          ^~~~~~~~~~~~~~
cc1.exe: all warnings being treated as errors
ninja: build stopped: subcommand failed.

I'll see if I can reproduce, but probably need more information on the environment being used here; it might be a very old compiler or something.

Don't know whether it's useful or not, but I recalled a patch which was adding missing includes: https://review.coreboot.org/c/flashrom/+/75235

could it be something similar in this case (missing includes)?

Yeah, that's all it looks like to me.

Miklos, could you please give us more info about your environment?

I know meson builds lots of info when it does a full re-build, but I don't see it in your output that you pasted. Maybe do a small change in the code and re-build? It will print:

[0/1] Regenerating build files.
The Meson build system
Version: 0.61.2
and then ~40 more lines about the environment on the machine.

That would be very useful.

Here's what my machine says when I build ni845x_spi only:

$ git checkout master
HEAD is now at d666a8189 doc: Fix broken link to old mailing list archives on pipermail
$ meson setup build -Dprogrammer=ni845x_spi
The Meson build system
Version: 1.1.1
Source dir: C:/Users/pmarheine/Desktop/flashrom
Build dir: C:/Users/pmarheine/Desktop/flashrom/build
Build type: native build
Project name: flashromutils
Project version: 1.4.0-devel
C compiler for the host machine: cc (gcc 13.1.0 "cc (Rev6, Built by MSYS2 project) 13.1.0")
C linker for the host machine: cc ld.bfd 2.40
Host machine cpu family: x86
Host machine cpu: x86
Program git found: YES (C:\msys64\usr\bin/git.EXE)
Program sphinx-build found: YES (C:\msys64\mingw32\bin/sphinx-build.EXE)
Compiler for C supports arguments -Wshadow: YES
Compiler for C supports arguments -Wmissing-prototypes: YES
Compiler for C supports arguments -Wwrite-strings: YES
Compiler for C supports arguments -Wno-unused-parameter: YES
Compiler for C supports arguments -Wno-address-of-packed-member: YES
Compiler for C supports arguments -Wno-enum-conversion: YES
Compiler for C supports arguments -Wno-missing-braces: YES
Checking for function "clock_gettime" : YES
Checking for function "strnlen" : YES
Check usable header "sys/utsname.h" : NO
Found pkg-config: C:\msys64\mingw32\bin/pkg-config.EXE (0.29.2)
Did not find CMake 'cmake'
Found CMake: NO
Run-time dependency libpci found: NO (tried pkgconfig and cmake)
Run-time dependency libusb-1.0 found: YES 1.0.26
Run-time dependency libftdi1 found: YES 1.5
Dependency libjaylink found: NO found 0.2.0 but need: '>=0.3.0'
Run-time dependency libjaylink found: NO (tried pkgconfig and cmake)
Library ni845x found: YES
Has header "linux/i2c.h" : NO
Checking for function "getopt_long" : YES
Run-time dependency bash-completion found: NO (tried pkgconfig and cmake)
Configuring flashrom.bash using configuration
Run-time dependency cmocka found: YES 1.1.5
Run-time dependency threads found: YES
Build targets in project: 7

flashromutils 1.4.0-devel

  Programmer
    active    : ni845x_spi
    non active: asm106x (not selected)
                atahpt (not selected)
                atapromise (not selected)
                atavia (not selected)
                buspirate_spi (not selected)
                ch341a_spi (not selected)
                ch347_spi (not selected)
                dediprog (not selected)
                developerbox_spi (not selected)
                digilent_spi (not selected)
                dirtyjtag_spi (not selected)
                drkaiser (not selected)
                dummy (not selected)
                ft2232_spi (not selected)
                gfxnvidia (not selected)
                internal (not selected)
                it8212 (not selected)
                jlink_spi (not selected)
                linux_mtd (not selected)
                linux_spi (not selected)
                parade_lspcon (not selected)
                mediatek_i2c_spi (not selected)
                mstarddc_spi (not selected)
                nic3com (not selected)
                nicintel (not selected)
                nicintel_eeprom (not selected)
                nicintel_spi (not selected)
                nicnatsemi (not selected)
                nicrealtek (not selected)
                ogp_spi (not selected)
                pickit2_spi (not selected)
                pony_spi (not selected)
                raiden_debug_spi (not selected)
                rayer_spi (not selected)
                realtek_mst_i2c_spi (not selected)
                satamv (not selected)
                satasii (not selected)
                serprog (not selected)
                stlinkv3_spi (not selected)
                usbblaster_spi (not selected)

  User defined options
    programmer: ni845x_spi

Found ninja-1.11.1 at C:\msys64\mingw32\bin/ninja.EXE
$ meson compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: C:\msys64\mingw32\bin/ninja.EXE -C C:/Users/pmarheine/Desktop/flashrom/build
ninja: Entering directory `C:/Users/pmarheine/Desktop/flashrom/build'
[115/115] Linking target tests/flashrom_unit_tests.exe

Using the default programmers (same procedure, but omit -Dprogrammer=...) also builds fine.

gcc (MinGW.org GCC-6.3.0-1) 6.3.0 is a very old compiler, and the "MinGW.org" line further supports that theory (that site no longer exists). The current documentation says to use msys2 which ships a much newer compiler (my environment is representative of that), so either you need to update your tools; or (if we decide that these old tools should still be supported) there need to be some code changes to work around the differences with an old standard library.

I also noticed the difference in two environments: in one of the env libusb and libftdi are not present.
Peter, is it possible that you try to uninstall these two libraries? maybe that would repro even with new compiler?

Build still succeeds for me after removing libusb, libftdi and libjaylink; confirmed that meson output says they were not found and many of the programmers were disabled since they need those libraries.

I am very confused, because all the includes seem to be present already.

fdopen and fileno are included with stdio.h
strdup is included in string.h

helpers_fileio.c includes "flash.h" -> which includes stdio.h
flashrom.c includes string.h (and also includes stdio.h)

Also in Miklos's logs, cmocka logs that both headers are present (check usable headers YES).

I am very curious how make builds it?

I think I found something.

There is an extra bit in Makefile which meson.build does not have, conditional for target OS MinGW:

# Some functions provided by Microsoft do not work as described in C99 specifications. This macro fixes that
# for MinGW. See http://sourceforge.net/p/mingw-w64/wiki2/printf%20and%20scanf%20family/ */
FLASHROM_CFLAGS += -D__USE_MINGW_ANSI_STDIO=1

I searched about __USE_MINGW_ANSI_STDIO and it seems to be deprecated? (thread from Jan 2019) https://osdn.net/projects/mingw/lists/archive/users/2019-January/000199.html

This would explain why the newer compiler works (because it knows about deprecation) and older one does not.

Miklos, as an experiment, you can try running meson with __USE_MINGW_ANSI_STDIO force enabled?
Hopefully this will do it:

meson configure builddir -D__USE_MINGW_ANSI_STDIO=1
or
meson setup builddir -D__USE_MINGW_ANSI_STDIO=1

In general, if the option is actually deprecated, I don't think we should be adding it to meson build?

Is it possible you would try to build with a newer compiler, as Peter suggested?

Thank you!

Nice sleuthing! I'd prefer to simply say that we now only support reasonably new compilers, but it seems potentially more correct to define the appropriate feature_test_macros(7). https://mingw-users.narkive.com/tAeczSOC/snprintf-and-use-mingw-ansi-stdio#post2 says you should never manually define __USE_MINGW_ANSI_STDIO and should use the appropriate feature test macros instead.

This turns out to be something I habitually skim right over in manpages, but it looks like always defining _XOPEN_SOURCE=500 is appropriate: the problematic functions identified earlier are fdopen, fileno and strdup. fdopen and fileno require _POSIX_C_SOURCE, and strdup requires (among other options) _XOPEN_SOURCE>=500. The glibc documentation says setting _XOPEN_SOURCE to a value 500 or greater implies _POSIX_C_SOURCE, so just setting _XOPEN_SOURCE is sufficient.

Setting this value expresses an expectation that the environment provides compliance with Single UNIX Specification v2 (1997), which seems like a reasonable assumption at this time: we already depend on these specifications by virtue of using those functions, and each of them is more than 20 years old at this time so I don't think it's worth bothering to support systems that fail to provide these features.

On further investigation, we already set _POSIX_C_SOURCE=200809 which should provide fdopen and fileno, so we can put the blame for those on MinGW (which seems like it has been fixed at some point). Pre-2.12 versions of glibc require _XOPEN_SOURCE >= 500 which we don't currently do, but that version was released in 2010 so I don't think it's worth supporting.

Missed a word: pre-2.12 versions of glibc require _XOPEN_SOURCE for strdup.

Peter Marheine wrote in #note-14:

it looks like always defining _XOPEN_SOURCE=500 is appropriate

Would that be a patch to meson.build (or tell me, maybe I am misunderstanding you)? There are few places which detect host_machine.system() , does this need to be added into one of those?
Is it useful to add this info into docs as well?

I meant in meson.build, but we already define a bunch of feature selection macros that should make the required features available. The problem is only that old versions of MinGW tools don't correctly handle those feature test macros to enable the requisite items, so we'd have to use __USE_MINGW_ANSI_STDIO if we want to support these old tools (and I don't think it's worth doing that).