[SSSD] [PATCH 6/7] Properly align buffer when storing pointers.

Michal Židek mzidek at redhat.com
Fri Dec 6 16:55:18 UTC 2013


On 12/05/2013 12:22 PM, Michal Židek wrote:
> On 12/03/2013 05:04 PM, Simo Sorce wrote:
>> On Tue, 2013-12-03 at 16:03 +0100, Michal Židek wrote:
>>> On 12/03/2013 02:34 PM, Simo Sorce wrote:
>>>> On Tue, 2013-12-03 at 13:28 +0100, Michal Židek wrote:
>>>>> On 12/03/2013 10:57 AM, Lukas Slebodnik wrote:
>>>>
>>>>>> I thik we can change while loop with som expresion.
>>>>>> pad = i %  sizeof(char *)
>>>>>> pad = (pad != 0) ? sizeof(char *) - pad : 0;
>>>>>>
>>>>>
>>>>> Ok, I added this change to the patch. I did not want to change the
>>>>> code
>>>>> much with these patches, the purpose was to just fix the alignment
>>>>> issues. But this is very small change and additional patch would just
>>>>> change the same lines. So I think it is ok to include.
>>>>>
>>>>>> or something nicer :-) (maybe macro?)
>>>>>
>>>>> I do not think that obfuscation with a macro is appropriate here.
>>>>
>>>> Actually the code is pretty hard to read and a macro would avoid
>>>> duplication and make it clear what the purpose is:
>>>>
>>>> Something like:
>>>>
>>>> #define PTR_SIZE sizeof(char *)
>>>> #define ALIGN_PTR_PAD(base, padding) \
>>>>       padding = (PTR_SIZE - (base % PTR_SIZE)) % PTR_SIZE)
>>>>
>>>> Then in the code just call:
>>>>
>>>> ALIGN_PTR_PAD(i, pad)
>>>
>>> I tend to dislike macros, but maybe we will need similar pattern on
>>> other places in the future as well, so putting this into macro may not
>>> be as bad idea as I originally though.
>>>
>>> However I would propose a different macro, that would be used like this:
>>> pad = CALCULATE_PAD(i, char *);
>>
>> Maybe PADDING_SIZE() then.
>>
>> Simo.
>>
>>
>
> Patch with changed macro name attached.
>
> Michal
>

I am sending new patch that will also suppress the alignment warning in 
sss_nss_mc_parse_result.

It introduces two simple macros IS_ALIGNED to check if address is 
aligned to sizeof(some_type)  and DISCARD_ALIGN that will internally 
cast the pointer to void* to suppress the warning.

If there is no bug, the buffer in this function should be properly 
aligned, however if the alignment is incorect, the function will fail 
with EFAULT. I originally wanted to do this check only for architectures 
that have problems with accessing unaligned memory, but then decided to 
do it for all platforms. The reason is, that we mostly test on x86 and 
we want to identify these issues as soon as possible if they occur.

New patch is attached.

Michal

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Properly-align-buffer-when-storing-pointers.patch
Type: text/x-patch
Size: 3572 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131206/5c4c3df1/attachment.bin>


More information about the sssd-devel mailing list