[SSSD] [PATCH] responder: Access packet header using SAFEALIGN macros

Jakub Hrozek jhrozek at redhat.com
Thu Nov 7 10:26:12 UTC 2013


On Tue, Nov 05, 2013 at 02:33:03PM +0100, Michal Židek wrote:
> On 11/05/2013 02:06 PM, Simo Sorce wrote:
> >On Tue, 2013-11-05 at 08:04 -0500, Simo Sorce wrote:
> >>The patch looks ok to me as is, however some of the function have become
> >>verbose because you used the raw copying macros.
> >>
> >>What about having setters and getters for cmd and status too ?
> >>
> >>Then sss_packet_get_cmd() becomes simply:
> >>{
> >>     return (enum sss_cli_command)sss_packet_get_cmd(packet);
> >>}
> >>which more closely matches the current code.
> >
> >ROTFL, and this is the classic example of rewriting your email while
> >reading the code and not proof reading it at the end :-)
> >
> >Ignore this part, just look at the next bit.
> >
> >>Also minor nack, the new functions you introduced are not namespaced.
> >>Please packet_get_len() -> sss_packet_get_len() and so on.
> >>
> >>I know these are static for now, but let's keep a naming consistent with
> >>sss_packet_set_error() and sss_packet_get_body() etc ...
> >>
> 
> I was thinking about whether to put those static functions to
> namespace or not. I decided not to do it in the end, but yes, it
> will probably be better if the static getters and setters share the
> same naming pattern
> with non static ones.
> 
> So new patch is attached (it is the same patch as before, just the
> function names are changed).
> 
> Michal
> 
> >>Simo.
> >>
> 

Simo's concern was addressed, the functions are now namespaced. Patch
works fine (getgrnam, initgroups, login, getpwnam).

ACK



More information about the sssd-devel mailing list