XrdClUtils :: RHEL7 build fails "attribute warn_unused_result [-Werror=unused-result]"

[zvada]

Build of xrootd-4.7.1-rc2 for RHEL7 platform in koji fails on declared with attribute warn_unused_result [-Werror=unused-result] -- a warning that gets turned into an error:

/builddir/build/BUILD/xrootd-4.7.1-rc2/src/./XrdCl/XrdClUtils.hh:282:31: error: ignoring return value of 'int setfsuid(__uid_t)', declared with attribute warn_unused_result [-Werror=unused-result]
           setfsuid(pPrevFsUid);
                               ^
[ 53%] /builddir/build/BUILD/xrootd-4.7.1-rc2/src/./XrdCl/XrdClUtils.hh:286:31: error: ignoring return value of 'int setfsgid(__gid_t)', declared with attribute warn_unused_result [-Werror=unused-result]
           setfsgid(pPrevFsGid);
                               ^

Bit of googling suggested workaround using cast to (void) to ignore it but including such patch doesn't help either and I see same build error:

/builddir/build/BUILD/xrootd-4.7.1-rc2/src/./XrdCl/XrdClUtils.hh:282:38: error: ignoring return value of 'int setfsuid(__uid_t)', declared with attribute warn_unused_result [-Werror=unused-result]
           (void) setfsuid(pPrevFsUid);
                                      ^
/builddir/build/BUILD/xrootd-4.7.1-rc2/src/./XrdCl/XrdClUtils.hh:286:38: error: ignoring return value of 'int setfsgid(__gid_t)', declared with attribute warn_unused_result [-Werror=unused-result]
           (void) setfsgid(pPrevFsGid);
                                      ^

Full build.log you can find here.

Build for RHEL6 works just fine.

Thanks,
Marian

[ljanyst]

Certain distributions of glibc seem to use __attribute__((warn_unused_result)) more aggressively than others. Function calls declared this way are not voidable. The rationale is here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509

[gbitzes]

Hello,

I'm not able to reproduce this issue, the build succeeds for me on RHEL 7. Instead of (void) setfsuid(pPrevFsUid);, could you try something like the following?

// ignore return value
if(setfsuid(pPrevFsUid)) { }

If this works for you, we can merge it.

[simonmichal]

I cannot reproduce it either (and we build with mock for all slc and fedora platforms, and with koji for slc 6 and 7). Is there anything specific about your koji build?

I wouldn't dwell on it too much, @gbitzes : could you simply add a log message (dump) in the destructor saying that the uid has been changed from x to y?

[gbitzes]

@simonmichal : Done, I added a commit to the existing pull request.

[simonmichal]

@zvada : could you try #605 ?

[zvada]

@simonmichal: which commit try out then? 08d4646 or cd33790?

[zvada]

Oh, per your comment about "log message (dump)" to @gbitzes I think this one should be tested cd33790. Or all files updated here need a test? Going to try out.

[simonmichal]

@zvada : actually cd33790 should fix your build problem, but you will also need 08d4646 to avoid ugly merge ;-)

[zvada]

I've put both commits into one patch - see this gist. And, scratch build for el7 passed now without an error!!!

Thanks,
Marian

[simonmichal]

Looks good :-)

Can we close this one, as the build failure has been addressed?

Michal

[zvada]

Yep, we can close. Do we get rc3 soon or all will go to official release cut soon? We have OSG development freeze on Monday, Oct 30. Would be great we can make non-RC release into the OSG schedule. Is it possible?

[simonmichal]

If this is the only issue with RC2 (and so far it is) then there's no need for RC3, I already ported those 2 commits to stable-4.7.x, by the end of the week, I'll cut the release.

[zvada]

We haven't had chance test it fully yet due block by build error but we are installing it now and should confirm whether all is good or not within a couple of days. Thanks!

[simonmichal]

OK, I'm waiting for a green light from you to do the release ;-)
I suppose we have time until Monday morning, if I do the release on Monday morning this would be enough for you due to the time difference, right?

[zvada]

Hi @simonmichal,

I believe I can confirm that issues addressed are gone after deploying rc2 in question. We've tested this on our StashCache instances at Nebraska and UChicago and also https+segfault issues for CMS@Florida didn't reappear (yet). :)

Please, do the release!

Thanks,
Marian

PS: Just a reminder include in the release also XrdUtils patch for the flawless SL7 build, please.

[xrootd-dev]

Hi Marian, Good to know. We will do a release as soon as possible. Right now we are trying to resolve some issues other testors have reported. So, the release may be delayed a bit. Andy

…

On Fri, 27 Oct 2017, Marian Zvada wrote: Hi @simonmichal, I believe I can confirm that issues addressed are gone after deploying rc2 in question. We've tested this on our StashCache instances at Nebraska and UChicago and also https+segfault issues for ***@***.*** didn't reappear (yet). :) Please, do the release! Thanks, Marian -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #606 (comment) ######################################################################## Use REPLY-ALL to reply to list To unsubscribe from the XROOTD-DEV list, click the following link: https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1

[zvada]

Hi Andy,

you mentioned in the other email thread you already have bug fix for it. Could this get into testing of yet another RC or are you guys considering make it available in official cut and when if so?

I'd like to see whether we can still push this into OSG release or not within few days. Even there is freeze scheduled for Oct 30th at OSG we could push this back a bit if we know what's your plan -- OSG definitely won't accept RC in the production release, however.

Thanks,
Marian

[xrootd-dev]

Hi Marian, Funny, we were just discussing this today. We definitely want to get this into 4.7.1 ASAP mostly to allow OSG to package the fixed release. We don't know the exact deadline OSG has but are wishfully thinking it won't be until midweek (will it -- can it get delayed until then?). We really would like to deploy the patch on one of our large scale testbeds (dozens of servers doing thousands of transactions) to make sure the fix really fixes this (I have high confidence it will). That won't happen until Monday and we would need to run at least 8 hours (the problem will actually show up within an hour if it's still there). So, that's the story. Andy

…

On Mon, 30 Oct 2017, Marian Zvada wrote: Hi Andy, you mentioned in the other email thread you already have bug fix for it. Could this get into testing of yet another RC or are you guys considering make it available in official cut and when if so? I'd like to see whether we can still push this into OSG release or not within few days. Even there is freeze scheduled for Oct 30th at OSG we could push this back a bit if we know what's your plan -- OSG definitely won't accept RC in the production release, however. Thanks, Marian -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #606 (comment) ######################################################################## Use REPLY-ALL to reply to list To unsubscribe from the XROOTD-DEV list, click the following link: https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1

[simonmichal]

Hi Marian,

I just created RC3, it's under testing now, We'll approximately need 1-2 days to test it.
So if everything is fine, I could do the release on Wednesday morning (Andy is confident
about his patch).

Is this OK for you?

Cheers,
Michal

[zvada]

Yes, that sounds good! I'll ask to make special on-hold in the OSG release process and wait till Wednesday for latest non-RC of xrootd. Crossing fingers that all tests go well. We may also try put RC3 in some of our tests in US, let's see.

Thanks,
Marian

[zvada]

@abh3, @simonmichal: assuming "XRootD 4.7.1 bugfix release" announced yesterday includes latest patches not previously tested in RC3 we did build from the final tag v4.7.1 for the coming OSG release as well. Just wanted to make sure this was the right course of action and things should be aligned with all recent code changes. Thanks.

[simonmichal]

@zvada : yes that's the right thing to do :-)
Just for the record, 4.7.1 includes just one patch that was not tested in RC3: 3c78db5
(which I tested in my testbed)

[zvada]

Perfect, that's what I thought. So we have built in OSG as well.