>-----Original Message----- >From: Mike Frysinger [mailto:[email protected]] >Sent: Monday, June 21, 2010 8:56 AM >To: Song, Barry >Cc: [email protected]; >[email protected] >Subject: Re: [Linux-kernel-commits] [8926] >trunk/fs/binfmt_flat.c: [!no_src_qa!] > >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. Why not you debug and check more carefully to find what happen? If the string is not in L1, how could helloworld can be printed in application by argv[1]. I have improved L1 test program to check this. > >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. I am sorry I set my vim to fix simple coding styling like this automatically....
>-mike > _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
