fails to build with -Werror=int-in-bool-context

[pohly]

The current version from meta-measured, 1.2.0, fails to compile with gcc 7.2.0 from OE-core Pyro, because:

| ../tpm2-tss/include/sapi/implementation.h:159:50: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
|  #define  CC_ECDH_ZGen                     (CC_YES*ALG_ECC)
|                                            ~~~~~~~^~~~~~~~~
| ../tpm2-tss/include/sapi/implementation.h:1271:23: note: in expansion of macro 'CC_ECDH_ZGen'
|         + (ADD_FILL || CC_ECDH_ZGen)                  /* 0x00000154 */ \
|                        ^~~~~~~~~~~~
| ../tpm2-tss/include/sapi/implementation.h:1338:6: note: in expansion of macro 'LIBRARY_COMMAND_ARRAY_SIZE'
|      (LIBRARY_COMMAND_ARRAY_SIZE + VENDOR_COMMAND_ARRAY_SIZE)
|       ^~~~~~~~~~~~~~~~~~~~~~~~~~
| ../tpm2-tss/include/sapi/tss2_tpm2_types.h:46:34: note: in expansion of macro 'COMMAND_COUNT'
|  #define    MAX_CAP_CC           (COMMAND_COUNT)
|                                   ^~~~~~~~~~~~~
| ../tpm2-tss/include/sapi/tss2_tpm2_types.h:1119:22: note: in expansion of macro 'MAX_CAP_CC'
|   TPM_CC commandCodes[MAX_CAP_CC];  /* a list of command codesThe maximum only applies to a command code list in a command. The response size is limited only by the size of the parameter buffer.  */
|                       ^~~~~~~~~~
| ../tpm2-tss/include/sapi/implementation.h:229:50: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
...

[martinezjavier]

@pohly yes, it has been fixed with commit 4e9c32ed9d21 ("configure: Add compiler flag to ignore int-in-bool-context errors from GCC 7."). It should be fixed in the upcoming 1.3 release.

For you you could give a try to 1.3.0-rc0.

[martinezjavier]

or you can of course change your bitbake recipe to include that compiler flag itself.

[pohly]

Good to know that you already found and addressed the issue. Personally I would have patched the source code instead of suppressing the error.

For the bitbake recipe I am indeed just changing the compile flags. That leaves meta-measured, which is currently broken for Pyro. Probably can be fixed by updating to 1.3.0 once it is available.

[martinezjavier]

@pohly the source code was in fact fixed in master by the following commits:

64862cb1f7a0 implementation.h: Remove preprocessor magic to selectively enable commands. 87feb17d81b8 implementation.h: Remove preprocessor magic to selectively enable algorithms.

I can't remember now why the solution was to add the compiler flag instead of cherry-picking those for the 1.x branch but probably @flihp can clarify.

[flihp]

I think this was a short term fix that became a long term fix. The issue that held this up is that these patches remove a number of things from public headers. If they're used by downstream consumers (unlikely IMHO) then we may end up breaking client code without breaking API or ABI.

So I've been conservative moving this stuff into a 1.x release. Disabling a warning was the solution / work-around with the lowest impact on clients. @martinezjavier you may be in the best position to answer this definitively since you work on / for a distro. How do you all view changes to headers like this?

[martinezjavier]

@flihp in general I would agree with you and be conservative on any changes to public headers between minor releases since this may be an API/ABI breakage. But my understanding from @williamcroberts changes is that what got removed was an indirection level that was only (supposed to be?) used by the SAPI library itself to disable some algorithms and commands.

But I may be wrong, since I didn't look in to much detail at the changes and I can't say that I completely understood what was the point of the ALG_{YES,NO} and CC_{YES,NO} in the first place and why someone would wanted to use it to not make some commands and algorithms visible to the client.

So in any case, his changes makes more things visible (that for some reasons were not) to the clients of the library.

[flihp]

Appreciate the input @martinezjavier. I'm going to keep these changes out of the 1.x releases since they remove a pile of macros that clients could be using. I don't see quieting a compiler warning as compelling enough to reason to risk breaking clients especially when we're already ignoring the warning though the build.

[martinezjavier]

@flihp yes, I agree with you.

[williamcroberts]

Just to be clear, I agree that these changes since they remove things should be kept out of the 1.X series. FYI, I back ported a small version of these changes (cast removal) to 1.X, but it doesn't break any API, see: #614