Siddhesh Poyarekar wrote: > On Mon, Jan 16, 2012 at 4:58 PM, Jamie Lokier <jamie@xxxxxxxxxxxxx> wrote: > > Is there a reason the names aren't consistent - i.e. not vma_is_stack_guard()? > > Ah, that was an error on my part; I did not notice the naming convention. > > > How about simply calling it vma_is_guard(), return 1 if it's PROT_NONE > > without checking vma_is_stack() or ->vm_next/prev, and annotate the > > maps output like this: > > > > is_stack => "[stack]" > > is_guard & is_stack => "[stack guard]" > > is_guard & !is_stack => "[guard]" > > > > What do you think? > > Thanks for the review. We're already marking permissions in the maps > output to convey protection, so isn't marking those vmas as [guard] > redundant? Yes it's redundant, I just think it's a bit clearer at showing the intent. After all that's also the reason for "[stack]", "[heap]" etc. It's not important though. > Following that, we could just mark the thread stack guard as [stack] > without any permissions. The process stack guard page probably > deserves the [stack guard] label since it is marked differently from > the thread stack guard and will otherwise have the permissions that > the process stack has. Will that be good? I don't have any strong opinions about what the test looks like; mainly I was pointing out the ->vm_next/prev check seemed dubious, that Glibc's layout shouldn't be assumed, and that hiding ranges from /maps may mislead some programs. Aesthetically I think if the main process stack has "[stack guard]", it makes sense for the thread stack guards to be labelled the same. One more technical thing: Now that you're using VM_STACK to change the text, why not set that flag for the process stack vma as well, when the stack is set up by exec, and get rid of the special case for process stack in printing? All the best, -- Jamie -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
