On 5/3/2011 1:39 AM, Kevin Cernekee wrote: > On Mon, May 2, 2011 at 9:44 AM, Carmelo AMOROSO <carmelo.amor...@st.com> > wrote: >> This patch is aimed to improve the aux vect handling to gather some useful >> information from the system through the aux vect. It is achieved by: > > Thanks for working on this - it looks like we're on the right track. > A few cosmetic nitpicks: > >> +/* Minimum number of AT entries from auxiliar vector */ > > "auxiliary" > >> +# error "AT_EXTRA_UPTO has to be defined (0 if not extra AT entried are >> needed)" > > "no extra AT entries" > >> +/* For the ld.so we are interested to the minimum entries from the auxvect >> */ > > "interested in" >
I should use spell check :) > It's a personal preference, but it might be a tiny bit more > straightforward to define libc AT_NUM in terms of AT_EXTRA_UPTO > directly. e.g. > > #define AT_BASE_NUM (AT_EGID + 1) > > # if AT_EXTRA_UPTO == 0 > # define AT_LIBC_NUM AT_BASE_NUM > # else > # define AT_LIBC_NUM (AT_EXTRA_UPTO + 1) > # endif > yes, it looks simpler. >> +extern size_t __pagesize; > > I see this getting declared as an extern in: dl-support.c, malloc.h, > getpagesize.c, and linuxthreads.old/internals.h . > > Do you think it would be a good idea to move the declaration into > <auxvect.h> or another common header file? > I don't like auxvect.h, but the idea to move into a common header is fine. >> - /* Get the number of program headers from the aux vect */ >> - _dl_phnum = (size_t) av[AT_PHNUM].a_un.a_val; >> + for (; av->a_type < AT_NUM; ++av) { >> + switch (av->a_type) { > > FWIW, that does result in mixed indentation styles in dl-support.c . > >> + for (; av->a_type < AT_NUM; ++av) { > > ... > >> + __pagesize = (size_t) (av[AT_PAGESZ].a_un.a_val) ? > > All of these cases should probably look more like "av->a_un.a_val", > since av gets incremented on each iteration of the loop. Without this > fix, the patch set does not work at all for me. > yes, my code is definitely wrong. I'll fix it and re-send. > There is another problem with setting __pagesize up in _dl_aux_init() > - it causes __uClibc_init() to skip initialization: > > void __uClibc_init(void) > { > /* Don't recurse */ > if (__pagesize) > return; > > The most obvious symptom that I noticed was that statically linked TLS > binaries crashed here: > > if (likely(not_null_ptr(__errno_location))) > *(__errno_location()) = 0; > ok, I have your patch for this and I'll merge it. >> +#ifndef SHARED >> + case AT_PHDR: >> + /* Get the program headers base address from the aux >> vect */ > > Maybe wouldn't hurt anything to leave these cases/variables enabled in > the SHARED builds? > for SHARED case, phdr stuff are retrieved in a different way. I'll check it in any case. Thanks, Carmelo _______________________________________________ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc