[SSSD] [PATCH] Use sss_strerror instead of strerror

Lukas Slebodnik lslebodn at redhat.com
Wed Aug 27 15:25:13 UTC 2014


On (27/08/14 11:08), Simo Sorce wrote:
>On Wed, 2014-08-27 at 17:00 +0200, Lukas Slebodnik wrote:
>> On (27/08/14 16:47), Jakub Hrozek wrote:
>> >On Wed, Aug 27, 2014 at 08:48:14AM -0400, Simo Sorce wrote:
>> >> On Wed, 2014-08-27 at 14:30 +0200, Jakub Hrozek wrote:
>> >> > On Wed, Jul 16, 2014 at 05:19:54PM +0200, Michal Židek wrote:
>> >> > > >We decided to use $(NULL) at the end of lists in Makefile.am. So you will not
>> >> > > >change two lines with adding new library next time.
>> >> > > >see 1746e8b8399da2a7a8da4aace186f66055ccfec1
>> >> > > >
>> >> > > >LS
>> >> > > 
>> >> > > Ok. I added it where I modified the end of the lists.
>> >> > > 
>> >> > > Michal
>> >> > > 
>> >> > 
>> >> > Simo,
>> >> > 
>> >> > I know you're normally against mass-converting anything in the code, but
>> >> > I think this conversion actually makes sense. We converted the DEBUG
>> >> > messages only recently with Nikolai's patches and strerror/sss_strerror
>> >> > is pretty much only used around DEBUG messages, so the context is
>> >> > already 'tainted'..
>> >> > 
>> >> > If you're OK with this mass-conversion, I'd like to push this patch to
>> >> > master and keep using sss_strerror only from now on.
>> >> 
>> >> You described my preferences quite well :)
>> >> 
>> >> My question is, given it seem we have to use sss_strerror() everywhere
>> >> why don't we simply ban the use of strerror() by redefining it to
>> >> sss_strerror() ?
>> >> If that would be too confusing I would like at least this macro be added
>> >> to the header file:
>> >> 
>> >> #idef strerror
>> >> #undef strerror
>> >> #endif
>> >> #define strerror(err) __ERROR__USE__SSS_STRERROR_INSTEAD__;
>> >> 
>> >> Simo.
>> >
>> >Simply because I don't like code obfuscation :-) If the code says it's
>> >calling strerror I'd like it to call strerror..
>> >
>> >But I don't feel too strongly about this issue, as long as the code
>> >prefers sss_strerror, I'm fine.
>> 
>> and you may forget to include such header file and you will use real function
>> from glibc.
>
>Should be in a header file that is included everywhere.
>
You cannot be sure, because there can be another library with the same HACK in
header file.

    //from our header file
#idef strerror
#undef strerror
#endif
#define strerror(err) __ERROR__USE__SSS_STRERROR_INSTEAD__;


   //from another header file which is included indirectly.
#idef strerror
#undef strerror
#endif
#define strerror(err) strerror_l((err), locale)

I just want to say that this kind of hacks are dangerous.

the 2nd problem is that Michal didn't replace all occurences of strerror.
He didn't replace strerror with sss_strerror after libc functions

For example:
src/util/domain_info_utils.c:433:                "fclose failed [%d][%s].\n", ret, strerror(ret));
src/util/domain_info_utils.c:508:              "fclose failed [%d][%s].\n", ret, strerror(ret));
src/util/domain_info_utils.c:516:              "rename failed [%d][%s].\n", ret, strerror(ret));
src/util/domain_info_utils.c:526:              "fchmod failed [%d][%s].\n", ret, strerror(ret));
src/util/domain_info_utils.c:544:                "fclose failed [%d][%s].\n", err, strerror(err));

LS



More information about the sssd-devel mailing list