[SSSD] [PATCH 1/7] sss_client: Use SAFEALIGN_COPY_<type> macros where appropriate

Michal Židek mzidek at redhat.com
Fri Nov 15 16:22:51 UTC 2013


On 11/14/2013 01:14 PM, Lukas Slebodnik wrote:> On (05/11/13 15:18),
Michal Židek wrote:
>>>>> Thanks for the responses.
>>>>>
>>>>> I decided to reduce the patches for this thread to only
>>>>> similar or simple changes. I think it is still quite enough
>>>>> for a review. I will post patches for the rest of the issues
>>>>> (like the one with
sockaddr) in
>>>>> separate threads. I hope this will make review easier. So
>>>>> after
applying
>>>>> these patches there will still be a lot of warnings.
>>>>>
>>>>> New patches attached.
>>>>>
>>>>> PS: Sorry for delayed answer.
>>>>>
>>>>>
>>>
>>> Sending patches rebased on top of current master.
>>>
>>> Michal
>>>
>> From 93a3b127e24ed26068ca7cfea41992a86440d8c2 Mon Sep 17 00:00:00
>> 2001
>>> From: Michal Zidek<mzidek at redhat.com> Date: Wed, 21 Aug 2013
>>> 17:17:06 +0200 Subject: [PATCH 1/7] sss_client: Use
>>> SAFEALIGN_COPY_<type> macros where diff --git
>>> a/src/sss_client/idmap/sss_nss_idmap.c
b/src/sss_client/idmap/sss_nss_idmap.c
>>> index e0faf6e..69f825f 100644 ---
>>> a/src/sss_client/idmap/sss_nss_idmap.c +++
>>> b/src/sss_client/idmap/sss_nss_idmap.c @@ -108,7 +108,7 @@ static
>>> int sss_nss_getyyybyxxx(union input inp,
enum sss_cli_command cmd ,
>>> goto done; }
>>>
>>> -    num_results = ((uint32_t *)repbuf)[0]; +
>>> SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL); if
>>> (num_results == 0) { ret = ENOENT; goto done; @@ -117,7 +117,7 @@
>>> static int sss_nss_getyyybyxxx(union input inp,
enum sss_cli_command cmd ,
>>> goto done; }
>>>
> Please add comment why comment why you skipped one unit32_t There is
> already similar comment on the other place in this patch. /* Skip
> first two 32 bit values (number of results and * reserved padding)
> */

Added.

>
>>> -    out->type = ((uint32_t *)repbuf)[2]; +
>>> SAFEALIGN_COPY_UINT32(&out->type, repbuf + 2 *
sizeof(uint32_t), NULL);
>>>
>>> data_len = replen - DATA_START;
>>>
>>> diff --git a/src/sss_client/nss_group.c
>>> b/src/sss_client/nss_group.c index e6ea54b..a7fb093 100644 ---
>>> a/src/sss_client/nss_group.c +++ b/src/sss_client/nss_group.c @@
>>> -298,7 +298,7 @@ enum nss_status _nss_sss_initgroups_dyn(const
char *user, gid_t group,
>>> }
>>>
>>> /* no results if not found */ -    num_ret = ((uint32_t
>>> *)repbuf)[0]; +    SAFEALIGN_COPY_UINT32(&num_ret, repbuf,
>>> NULL); if (num_ret == 0) { free(repbuf); nret =
>>> NSS_STATUS_NOTFOUND; @@ -328,9 +328,13 @@ enum nss_status
>>> _nss_sss_initgroups_dyn(const
char *user, gid_t group,
>>> *size = newsize; }
>>>
> The same issue is here.
>>> -    rbuf = &((uint32_t *)repbuf)[2]; +    /* Skip first two 32
>>> bit values (number of results and +     * reserved padding) */
            ^^^^^^^^^^^^^^^^^^^^^
The comment was there. Maybe I missed something?

>>> +    buf_index = 2 * sizeof(uint32_t); + for (l = 0; l < max_ret;
>>> l++) { -        (*groups)[*start] = rbuf[l]; +
>>> SAFEALIGN_COPY_UINT32(&(*groups)[*start], repbuf + buf_index, +
>>> &buf_index); *start += 1; }
>>>
> Otherwise, conditions look much nicer with variables like a
> "num_results" instead of "buffer magic" (((uint32_t *)repbuf)[0] ==
> 0)


New patch attached.

Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sss_client-Use-SAFEALIGN_COPY_-type-macros-where-app.patch
Type: text/x-patch
Size: 14194 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131115/1f10d746/attachment.bin>


More information about the sssd-devel mailing list