[SSSD] [PATCH] responder: Access packet header using SAFEALIGN macros
Michal Židek
mzidek at redhat.com
Tue Nov 5 13:33:03 UTC 2013
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.
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-responder-Access-packet-header-using-SAFEALIGN-macro.patch
Type: text/x-patch
Size: 7071 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131105/77563af1/attachment.bin>
More information about the sssd-devel
mailing list