Messages in this thread | | | Date | Wed, 4 Mar 2009 16:48:04 -0500 | Subject | Re: [patch -v2] flat: fix data sections alignment | From | Mike Frysinger <> |
| |
On Wed, Mar 4, 2009 at 08:51, Johannes Weiner wrote: > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > +#ifdef ARCH_SLAB_MINALIGN > +#define FLAT_DATA_ALIGN (ARCH_SLAB_MINALIGN) > +#else > +#define FLAT_DATA_ALIGN (sizeof(void *)) > +#endif > ... > + sp = (unsigned long *) ((unsigned long)sp & -FLAT_DATA_ALIGN); > ... > - datapos = realdatastart + MAX_SHARED_LIBS * sizeof(unsigned long); > + datapos = ALIGN(realdatastart + MAX_SHARED_LIBS * sizeof(unsigned long), > + FLAT_DATA_ALIGN); > ... > - datapos = realdatastart + MAX_SHARED_LIBS * sizeof(unsigned long); > + datapos = ALIGN(realdatastart + MAX_SHARED_LIBS * sizeof(unsigned long), > + FLAT_DATA_ALIGN);
why not make FLAT_DATA_ALIGN into a macro ? then it'd naturally follow the existing ALIGN() behavior.
> - sp = (unsigned long *) ((-(unsigned long)sizeof(char *))&(unsigned long) p); > - > - sp -= envc+1; > - envp = sp; > - sp -= argc+1; > - argv = sp; > + sp = (unsigned long *)p; > + sp -= (envc + argc + 2) + 1 + (flat_argvp_envp_on_stack() ? 2 : 0); > + sp = (unsigned long *) ((unsigned long)sp & -FLAT_DATA_ALIGN); > + argv = sp + 1 + (flat_argvp_envp_on_stack() ? 2 : 0); > + envp = argv + (argc + 1);
can this be cleaned up a bit so that the argv/envp assignment happens by using sp before aligning sp ? that would be defensive coding wrt preventing sp adjustment falling out of line with argv initialization, and cut down on duplicated code.
> - put_user(argc,--sp); > + put_user(argc,sp);
might as well fix the style issues here while you're changing code ... stick a space after the comma
> @@ -854,7 +861,7 @@ static int load_flat_binary(struct linux > stack_len = TOP_OF_ARGS - bprm->p; /* the strings */ > stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */ > stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ > - > + stack_len += FLAT_DATA_ALIGN;
this seems weird. alignment is for aligning data, not padding it out some value ... -mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |