Bug 102145 - -Werror=declaration-after-statement build failure on Solaris
Summary: -Werror=declaration-after-statement build failure on Solaris
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: Other Solaris
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard: review+
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-11 04:36 UTC by Alan Coopersmith
Modified: 2017-08-16 17:05 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
patch to fix build error (2.02 KB, patch)
2017-08-11 04:36 UTC, Alan Coopersmith
Details | Splinter Review

Description Alan Coopersmith 2017-08-11 04:36:57 UTC
Created attachment 133433 [details] [review]
patch to fix build error

dbus fails to build from git master (commit 52aeb92f9a37309ae34f55f) 
with gcc 5.4 on Solaris due to:

dbus-sysdeps-unix.c: In function ‘_dbus_read_credentials_socket’:
    dbus-sysdeps-unix.c:2061:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
             adt_session_data_t *adth = NULL;
             ^

Attached patch fixes that by moving variable declarations.
Comment 1 Philip Withnall 2017-08-11 17:15:00 UTC
Comment on attachment 133433 [details] [review]
patch to fix build error

Review of attachment 133433 [details] [review]:
-----------------------------------------------------------------

Looks good to me. r+
Comment 2 Simon McVittie 2017-08-15 16:19:53 UTC
Looks good to me too.

Alan, if you know what ADT is, please could you write a brief description as a patch to the D-Bus Specification (doc/dbus-specification.xml) with a typical value or a hint pointing users to some relevant OS functions? The current documentation is

    Returns auditing data used by Solaris ADT, in an unspecified binary
    format. If you know what this means, please contribute documentation
    via the D-Bus bug tracking system.

which is obviously not ideal!

It would also be great if you could add this field to the GetConnectionCredentials() method, as a new OS-dependent field similar to the current LinuxSecurityLabel and WindowsSID. GetConnectionCredentials() is the modernized replacement for GetConnectionUnixUser(), GetConnectionSELinuxContext(), GetAdtAuditSessionData() and all other credentials-querying methods.
Comment 3 Simon McVittie 2017-08-15 16:24:21 UTC
Thanks for the patch; fixed in git for 1.10.24 and 1.11.18. Please open a separate bug for any ADT-related enhancements.
Comment 4 Alan Coopersmith 2017-08-15 17:51:59 UTC
ADT is the API for recording events in the Solaris Audit Trail, described in
http://www.oracle.com/pls/topic/lookup?ctx=solaris11&id=OSMAA and
http://us-east.manta.joyent.com/jmc/public/opensolaris/ARChive/PSARC/2000/517/incept.materials/bsmAPI.html#adt_export_session_data

I'll look at updating to the current API & documenting when I have a chance.
Comment 5 Simon McVittie 2017-08-16 17:05:14 UTC
(In reply to Alan Coopersmith from comment #4)
> I'll look at updating to the current API & documenting when I have a chance.

Thanks! The links you provided are probably useful (I will admit I haven't actually followed them) but I am not going to be documenting or enhancing this feature myself, because I have no way to test it. I suspect the same goes for all the dbus maintainers. I'm happy to review tested patches, though.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.