Re: [exim-dev] constness fix: please review

Top Page
Delete this message
Reply to this message
Author: Tom Kistner
Date:  
To: 'Phil Pennock', exim-dev
Subject: Re: [exim-dev] constness fix: please review
Sorry Phil, I saw this message too late and also commited a fix, but for
strsignal() only. Yours should go in instead, I'll back out mine.

/tom


-----Original Message-----
From: exim-dev-bounces@??? [mailto:exim-dev-bounces@exim.org] On Behalf
Of Phil Pennock
Sent: Samstag, 26. März 2011 05:45
To: exim-dev@???
Subject: [exim-dev] constness fix: please review

Tom Kistner reported that my constness updates from the "Compiler masochism
compliance" patch:

http://git.exim.org/exim.git/commit/1ba28e2b955b005ce4825fec792df17f75a8de1e
broke build on Linux; in his words:

> I was fiddling with the latest git state today. The "const"ification
> in src/osfunctions.h breaks build on my Linux box, because
> OS/os.h-Linux sets "#define os_strsignal strsignal". Then the
> prototype in /usr/include/string.h and aliased prototype in
> src/osfunctions.h have conflicting types


On the whole, I'm still happy with the change and don't want to revert it,
since it clears the way for more stringent compiler warnings which should
result in less buggy code. Heck, it led to my spotting the
ldap_require_cert bug.

I think that the fix is sufficiently simple to warrant patching rather than
reversion.

My proposed fix is:

http://git.exim.org/users/pdp/exim.git/commit/b52ce0604579f50b71e7527e5030df
b3f74c618b

----------------------------8< cut here >8------------------------------
Rely on system prototypes if we #define our os funcs.

The const-ness updates broke systems where `os_strsignal()` gets mapped to
`strsignal()`, which does *not* return `const char *` but `char *`.

If we #define away, then there should be a prototype from the system
headers.
----------------------------8< cut here >8------------------------------

Does this generally look sane to people? I temporarily modified
os.h-FreeBSD to use the same strsignal() redefine and without the commit
above, things broke but with the patch above, things compile.

I'm about to shut down my laptop to pack it for a plane flight, so I'm not
going to push changes to the master repo when I'm not going to be around to
deal with fall-out. If someone else is happy with this change, feel free to
push it to the master repo. :)

Sorry for the breakage, and thanks for the report, Tom.

-Phil