[PATCH v3] libsframe: testsuite: Fix testsuite build on Solaris

[Indu Bhagat]

On 9/10/25 12:32 AM, Rainer Orth wrote:
> Hi Indu,
> 
>>> Tested on {i386,amd64}-pc-solaris2.11, {sparc,sparcv9}-sun-solaris2.11,
>>> {x86_64,i686}-pc-linux-gnu, and amd64-pc-freebsd14.0.
>>> Coauthored-By: Alan Modra<amodra@gmail.com>
>>> Ok for trunk?
>>
>> Patch looks good to me.  Two comments inlined below.
> 
> thanks.
> 
>> Thanks for doing this.  This being your work/fix, I've reassigned the PR
>> 33168 to you.  It will be good to add a "PR libsframe/33168" to the commit
>> log for marking the fix.
> 
> That's one thing I wondered about: libsframe currently doesn't have no
> ChangeLog file.  Unlike over in gcc, that's not generated from the
> commit messages.  So far I missed that the libsfram commit message *do*
> include ChangeLog snippets.  I'll add one to the commit, together with
> the PR reference.
> 

Thanks.

BTW, Yes, I tend to include ChangeLog snippets in my own commits, as I 
find them helpful.  This is _not_ mandatory wrt Binutils commit log 
guidelines.

> Maybe it would be best to follow the rest of binutils and start manually
> adding the entries to an actual ChangeLog file?  

It is my understanding that in Binutils, it is not required to add 
entries to a ChangeLog file anymore.

But that said, adding a ChangeLog snippet in commit log is orthogonal to 
adding a "PR libsframe/33168" to commit log.  The post commit hooks will 
DTRT with just that string in the commit log.

That would be less
> confusing and make the project consistent in this regard.  However, I'd
> actually prefer if binutils could just follow gcc's lead and extract the
> info from the commit messages: it avoids the cumbersome and easily
> forgotten step of manually adding them at commit time.
> >>> diff --git a/libsframe/testsuite/sframe-test.h 
b/libsframe/testsuite/sframe-test.h
>>> new file mode 100644
>>> --- /dev/null
>>> +++ b/libsframe/testsuite/sframe-test.h
>>> @@ -0,0 +1,40 @@
>>> +/* sframe-test.h -- Common libsframe testsuite header.
>>> +
>>> +   Copyright (C) 2022-2025 Free Software Foundation, Inc.
>>> +
>>
>> Typo: 2022-2025 --> 2025
> 
> Really?  The bulk of the file is just copied from existing code.
> Shouldn't it retain that code's date range?
> 

I usually follow the rule of keeping the current year for a new file, 
since the file did not exist prior to date.  May be someone else can 
confirm whats more appropriate here.

>>> +/* PR 33168: Solaris <sys/wait.h> with __EXTENSIONS__ defined gets a
>>> +   declaration of wait().  This conflicts with <dejagnu.h>'s wait
>>> +   definition that is only removed after DejaGnu 1.6.3.  */
>>> +#define wait dg_wait
>>> +#include <dejagnu.h>
>>> +#undef wait
>>> +
>>> +#endif /* _SFRAME_TEST_H */
>>>
>>
>> Although the -Wall for testsuite was redundant, just wanted to point out
>> that removing it is not enough to address the current warnings seen:
>>
>> In file included from
>> <src-path>/libsframe/testsuite/libsframe.decode/frecnt-1.c:18:
>> <src-path>/libsframe/testsuite/sframe-test.h:36:14: warning: no previous
>> prototype for ‘dg_wait’ [-Wmissing-prototypes]
>>     36 | #define wait dg_wait
>>        |              ^~~~~~~
> 
> Sorry I missed that: the make check is usually too verbose to check on
> anything but complete failures.
> 
>> Personally, I find the warning distracting in the logs. The buildbot does
>> seem to track warnings for testsuite too, so it might complain about the
>> new warnings in the testsuite.
> 
> Agreed.
> 
>> If adding a prototype "void wait (void);" is not preferable, we can add
>> -Wno-missing-prototypes for the testsuite ?
> 
> I prefer the prototype route over the warning myself if it works.
> 
> Should I just commit the final patch with those updates or post a v4?
> 

Please go ahead and commit.  Thanks.