On Fri, Dec 06, 2013 at 11:58:38AM -0500, Simo Sorce wrote: > On Fri, 2013-12-06 at 17:55 +0100, Michal Židek wrote: > > 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. > > Thank you Michal, > it looks good! > > Ack from me (but I haven't actually tested the code).
I ran a couple of sanity tests and haven't seen anything broken. I'm going to push. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
