URL: https://github.com/SSSD/sssd/pull/231
Title: #231: changing all talloc_get_type() with talloc_get_type_abort()
simo5 commented:
"""
So I can't really comment inline because there are too many lines in the patch
but I grepped throught the code:
I see:
+ void sss_talloc_abort(const char reason){
Well .. that should be a const char * to start with as you want to pass a
pointer to string not a single character
Also the style is incorrect as the opening brace should be on the next line.
The content of the function also is not what the issue describes.
the issue describes the option to either abort() or print a debug error based
on configuration (or perhaps build flags). Instead this code unconditionally
prints a debug error (this may be ok actually) and then unconditionally calls
abort().
Finally this patchset is going to be big as it touches thousands of file in a
mostly mecahnical way.
To improve reviewability it wuld be nice to split this PR in 2/3 patches:
1. Introduce the new sss_talloc_abort() function in the first patch with a good
commit message that explains what this will bring us.
2. a separate patch that enables it everywhere
3. a separate patch that changes the invocations ofa talloc_get_type() to
talloc_get_type_abort()
HTH.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/231#issuecomment-292973967
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
