[LU-13274] Building againt lustreapi using -std=c99

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.14.0, Lustre 2.12.5
• Type: Improvement
• Priority: Minor
• Reporter: Gian-Carlo Defazio
• Assignee: James A Simmons
• Resolution: Fixed
• Labels: llnl

Description

An application that uses the compiler flag -std=c99 wants to link against lustreapi. This was possible in version 2.8. Current versions of lustreapi use the linux kernel macro __ALIGN_KERNEL() which contains the gnuism typeof, as opposed the ISO version __typeof__. For older kernels that don't define ___ALIGN_KERNEL(), Lustre conditionally defines it.

The "simplest" fix would be for the application to add -Dtypeof=__typeof__, however, this would require figuring out where in the build system to add such a flag. The other option would be to change Lustre and redefine typeof to __typeof__ in the source code of the files that use __ALIGN_KERNEL() or maybe somewhere in the Lustre build system.

Are there assumptions about the C version that is used by applications that use lustreapi?

 

 

 

Issue Links

• is related to: LU-12511 Prepare lustre for adoption into the ...
• is related to: LU-13315 RHEL8 build failed with some options
• is related to: LU-13525 struct sepol_downcall_data is badly f...

Comments

[James A Simmons]

This is strange. That means any file including kernel.h breaks with std99. That includes things like sysctl.h. I will have to look how other projects work around it.

[James A Simmons]

Can you try adding #include <stddef.h> and see if it works then

[Gian-Carlo Defazio]

Adding #include <stddef.h> didn't change anything. typeof is still not recognized.

 

The test program I'm using is 

#include <stddef.h>
#include <lustre/lustreapi.h>
int main(int argc, char* argv[]) {
        return 0;
}

 

And I get this output, with or without <stddef.h>

gcc -DHAVE_CONFIG_H -I. -I../..   -include /g/g0/defazio1/lustre-release/undef.h -include /g/g0/defazio1/lustre-release/config.h -I/g/g0/defazio1/lustre-release/libcfs/include -I/g/g0/defazio1/lustre-release/lnet/include -I/g/g0/defazio1/lustre-release/lnet/include/uapi -I/g/g0/defazio1/lustre-release/lustre/include -I/g/g0/defazio1/lustre-release/lustre/include/uapi  -fPIC -D_GNU_SOURCE -D_LARGEFILE64_SOURCE=1 -D_FILE_OFFSET_BITS=64 -DLUSTRE_UTILS=1  -std=c99 -g -O2 -Wall -Werror -MT test_param_get_values-test_param_get_values.o -MD -MP -MF .deps/test_param_get_values-test_param_get_values.Tpo -c -o test_param_get_values-test_param_get_values.o `test -f 'test_param_get_values.c' || echo './'`test_param_get_values.c
In file included from /g/g0/defazio1/lustre-release/lustre/include/lustre/lustreapi.h:44:0,
                 from test_param_get_values.c:26:
/g/g0/defazio1/lustre-release/lustre/include/uapi/linux/lustre/lustre_user.h: In function 'hai_first':
/g/g0/defazio1/lustre-release/lustre/include/uapi/linux/lustre/lustre_user.h:2321:2: error: implicit declaration of function 'typeof' [-Werror=implicit-function-declaration]
  size_t offset = __ALIGN_KERNEL(strlen(hal->hal_fsname) + 1, 8);

[James A Simmons]

Ouch I see many failures. This kind of overlaps with the issues I see with the native linux lustre client and its UAPI headers. If you enable CONFIG_UAPI_HEADER_TEST you see these kinds of errors. Let me see if I can fix things up.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37678
Subject: LU-13274 uapi: make lustre UAPI headers C99 compliant
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 3e7974482a1025dcdbefcd11844bb50cd972df86

[James A Simmons]

We still have LNet UAPI headers to fix

[Gian-Carlo Defazio]

Thanks, I was able to compile my minimal test program using your patch.

[Gian-Carlo Defazio]

I commented on code review, but this is the more appropriate place...

With patch 2 I can compile my dummy program with -std=c99, which just checks that the lustreapi.h header can be included. I was not able to compile liblustreapi.o with patch 2 using -std=c99, but users wouldn't do that anyways so I don't think it matters.

I see that there's more going on than just changing typeof to _typeof_ depending on compiler flags. Are you fixing other potential C version errors? Or is the code rearrangement for some other reason?

[James A Simmons]

In out test suite we have 2 test that make sure you can build user land applications using our library header as well has our UAPI kernel headers. When I added the std=c99 option to gcc a bunch of errors came out. So my patch address all the issues. The nice thing about updating our internal test that it makes sure any new code landed will actually work with std=c99. As for building our user land software stack I have to check that out. Never done that.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37678/
Subject: LU-13274 uapi: make lustre UAPI headers C99 compliant
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 7a7309fa849577ddd5a4f6bb5bfb69e84a7fec89

[James A Simmons]

More patches coming.

[Olaf Faaland]

> More patches coming.

James,
What do the other patches fix?
Thanks

[Olaf Faaland]

Does this merit a backport to b2_12 in general?  It does for us.

[James A Simmons]

LNet UAPI headers and one more round of fixes for Lustre UAPI headers. I have a patch for LNet already but haven't made a maloo test for it.

For the Lustre UAPI header fixes its the fallout seen from CONFIG_UAPI_HEADER_TEST.  Its pretty straight forward except for some reason the linux kernel does like the use of snprintf() in hai_dump_data_field(). Haven't figured out a solution yet.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37971
Subject: LU-13274 uapi: make lnet UAPI headers C99 compliant
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 6534401add98d741807cd44f1af4152c1ce14eed

[Gerrit Updater]

Gian-Carlo DeFazio ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37973
Subject: LU-13274 uapi: make lustre UAPI headers C99 compliant
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: 98c09a4a057d05500e739a4e6f128661a74ad713

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37973/
Subject: LU-13274 uapi: make lustre UAPI headers C99 compliant
Project: fs/lustre-release
Branch: b2_12
Current Patch Set:
Commit: 0417dce9fc75ad8368b6a60fb8dd9556254e2741

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37971/
Subject: LU-13274 uapi: make lnet UAPI headers C99 compliant
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 742897a967cff5be53c447d14b17ae405c2b31f2

[Peter Jones]

simmonsja is there more to come on this ticket?

[James A Simmons]

One last patch is coming.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/38215
Subject: LU-13274 uapi: make lustre UAPI headers pass CONFIG_UAPI_HEADER_TEST
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 1d1f901b6ddc155832533f891cf24eba16b0ccd9

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/38215/
Subject: LU-13274 uapi: make lustre UAPI headers pass CONFIG_UAPI_HEADER_TEST
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: b93ef29660b6ac69d7ffd0f325be02c18835b4d2

[Gerrit Updater]

Andreas Dilger ([e-mail]) uploaded a new patch: https://review.whamcloud.com/38520
Subject: LU-13274 uapi: fix build on older kernels
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: ead42b98dcef1844cc4790227c7ad1b58b88594d

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/38520/
Subject: LU-13274 uapi: fix build on older kernels
Project: fs/lustre-release
Branch: b2_12
Current Patch Set:
Commit: fd34cb106a601cf1c71cfb46a9887a5d0ba2e46f