On Sun, Jun 20, 2010 at 06:12, Song, Barry wrote: >From: Mike Frysinger [mailto:[email protected]] >>On Fri, Jun 18, 2010 at 04:33, <[email protected]> wrote: >>> [#6034], fix argv and envp pointer while using L1 stack >>> >>> Modified: trunk/fs/binfmt_flat.c (8925 => 8926) >>> >>> put_user(argc, sp); >>> current->mm->arg_start = (unsigned long) p; >>> while (argc-->0) { >>> +#ifndef CONFIG_APP_STACK_L1 >>> put_user((unsigned long) p, argv++); >>> +#else >>> + put_user((unsigned long) p + l1_ram_offset, argv++); >>> +#endif >>> ... >>> while (envc-->0) { >>> - put_user((unsigned long)p, envp); envp++; >>> +#ifndef CONFIG_APP_STACK_L1 >>> + put_user((unsigned long)p, envp++); >>> +#else >>> + put_user((unsigned long)p + l1_ram_offset, envp++); >>> +#endif >> >>i dont think we want this behavior. the argv/envp can easily exceed >>the tiny 4k scratchpad, not to mention the app itself exceeding it. > > The origin codes actually have copied argv and env to L1 stack. > Following the old logic, adjusting SP+1 is the right way to a fast fix. > Then you mean we can place argv and env in ram and give a legal VMA to > cover it? That will make a difference and mean a new small task, > not a bug fix.
the changes you made that were needed to make this work are overly invasive. if we cant support L1 stack in a simple way, then we need a different solution. perhaps the adding of all the #ifdef makes it worse than it should be. why are you checking CONFIG_APP_STACK_L1 at all ? the core logic already takes care of ignoring the L1 stack flag in the FLAT header when this define is enabled which means all those other #ifdefs are redundant. looking again, you're just putting the array of pointers onto the stack and not the strings they point to right ? i was thinking you were putting the strings themselves on there, but the arrays should be OK as those shouldnt take up too much space. perhaps we show people how to create a custom main() that sets the stack to a custom location before calling the real main. also, i noticed that a lot of the changes here are simply whitespace related. while the old way was incorrect, it isnt really our business to go fixing them in common code let alone mixing real commits with whitespace shuffling. -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
