----- Original Message -----
> From: "Wanlong Gao" <[email protected]>
> To: "Jan Stancek" <[email protected]>
> Cc: [email protected], "Caspar Zhang" <[email protected]>, 
> "Cyril Hrubis" <[email protected]>
> Sent: Monday, 29 October, 2012 3:18:29 AM
> Subject: Re: [LTP] [PATCH v2] ipc/shmat: pick suitable base_addr before each 
> testcase
> 
> On 10/26/2012 07:20 PM, Jan Stancek wrote:
> > This test picked fixed address in setup() which was used throughout
> > the test. There has been reports that it can fail with EINVAL.
> > 
> > I could reproduce it by adding single printf in between
> > detach/attach:
> > http://article.gmane.org/gmane.linux.ltp/16480
> > 
> > This patch picks suitable base_addr before each test. So even if
> > glibc
> > would mmap extra page with addr == base_addr picked by setup(),
> > new suitable address will be chosen.
> > 
> > Signed-off-by: Jan Stancek <[email protected]>
> > ---
> >  testcases/kernel/syscalls/ipc/shmat/Makefile       |    3 ++
> >  testcases/kernel/syscalls/ipc/shmat/shmat01.c      |   16
> >  ++-------
> >  testcases/kernel/syscalls/ipc/shmat/shmat02.c      |   16
> >  ++-------
> >  testcases/kernel/syscalls/ipc/shmat/shmat_common.c |   34
> >  ++++++++++++++++++++
> >  4 files changed, 43 insertions(+), 26 deletions(-)
> >  create mode 100644
> >  testcases/kernel/syscalls/ipc/shmat/shmat_common.c
> > 
> > diff --git a/testcases/kernel/syscalls/ipc/shmat/Makefile
> > b/testcases/kernel/syscalls/ipc/shmat/Makefile
> > index f467389..5cbf37c 100644
> > --- a/testcases/kernel/syscalls/ipc/shmat/Makefile
> > +++ b/testcases/kernel/syscalls/ipc/shmat/Makefile
> > @@ -20,4 +20,7 @@ top_srcdir              ?= ../../../../..
> >  
> >  include $(top_srcdir)/include/mk/testcases.mk
> >  include $(abs_srcdir)/../Makefile.inc
> > +
> > +MAKE_TARGETS            := $(patsubst
> > $(abs_srcdir)/%.c,%,$(wildcard $(abs_srcdir)/*[0-9].c))
> > +
> >  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat01.c
> > b/testcases/kernel/syscalls/ipc/shmat/shmat01.c
> > index 6e4dee1..160abca 100644
> > --- a/testcases/kernel/syscalls/ipc/shmat/shmat01.c
> > +++ b/testcases/kernel/syscalls/ipc/shmat/shmat01.c
> > @@ -56,6 +56,7 @@
> >   */
> >  
> >  #include "ipcshm.h"
> > +#include "shmat_common.c"
> >  
> >  char *TCID = "shmat01";
> >  
> > @@ -108,6 +109,7 @@ int main(int ac, char **av)
> >                     /*
> >                      * Use TEST macro to make the call
> >                      */
> > +                   base_addr = probe_free_addr();
> >                     errno = 0;
> >                     addr = shmat(*(TC[i].shmid), base_addr+TC[i].offset,
> >                                  TC[i].flags);
> > @@ -262,19 +264,7 @@ void setup(void)
> >                      "resource 1 in setup()");
> >     }
> >  
> > -   /* Probe an available linear address for attachment */
> > -   if ((base_addr = shmat(shm_id_1, NULL, 0)) == (void *)-1) {
> > -           tst_brkm(TBROK, cleanup, "Couldn't attach shared memory");
> > -   }
> > -   if (shmdt((const void *)base_addr) == -1) {
> > -           tst_brkm(TBROK, cleanup, "Couldn't detach shared memory");
> > -   }
> > -
> > -   /* some architectures (e.g. parisc) are strange, so better always
> > align to
> > -    * next SHMLBA address. */
> > -   base_addr =
> > -       (void *)(((unsigned long)(base_addr) + (SHMLBA - 1)) &
> > -                ~(SHMLBA - 1));
> > +   base_addr = probe_free_addr();
> 
> We don't need to get base_addr in setup(), right? seems get twice.

Probably not, I left it there just to exercise that codepath too before actual
test.

> 
> >  }
> >  
> >  /*
> > diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat02.c
> > b/testcases/kernel/syscalls/ipc/shmat/shmat02.c
> > index d4cca0e..b260e18 100644
> > --- a/testcases/kernel/syscalls/ipc/shmat/shmat02.c
> > +++ b/testcases/kernel/syscalls/ipc/shmat/shmat02.c
> > @@ -55,6 +55,7 @@
> >  
> >  #include "ipcshm.h"
> >  #include <pwd.h>
> > +#include "shmat_common.c"
> >  
> >  char *TCID = "shmat02";
> >  char nobody_uid[] = "nobody";
> > @@ -125,6 +126,7 @@ int main(int ac, char **av)
> >  
> >                     setup_tc(i, tc);
> >  
> > +                   base_addr = probe_free_addr();
> >                     errno = 0;
> >                     addr = shmat(*(tc->shmid), base_addr + tc->offset, 0);
> >  
> > @@ -181,19 +183,7 @@ void setup(void)
> >         -1)
> >             tst_brkm(TBROK|TERRNO, cleanup, "shmget #2 failed");
> >  
> > -   /* Probe an available linear address for attachment */
> > -   if ((base_addr = shmat(shm_id_2, NULL, 0)) == (void *)-1)
> > -           tst_brkm(TBROK|TERRNO, cleanup, "shmat #1 failed");
> > -
> > -   if (shmdt((const void *)base_addr) == -1)
> > -           tst_brkm(TBROK|TERRNO, cleanup, "shmat #2 failed");
> > -
> > -   /*
> > -    * some architectures (e.g. parisc) are strange, so better always
> > align
> > -    * to next SHMLBA address
> > -    */
> > -   base_addr =
> > -       (void *)(((unsigned long)(base_addr) & ~(SHMLBA - 1)) +
> > SHMLBA);
> > +   base_addr = probe_free_addr();
> 
> ditto
> 
> >  }
> >  
> >  /*
> > diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat_common.c
> > b/testcases/kernel/syscalls/ipc/shmat/shmat_common.c
> > new file mode 100644
> > index 0000000..508bf4b
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/ipc/shmat/shmat_common.c
> > @@ -0,0 +1,34 @@
> > +static key_t probe_key;
> > +
> > +void *probe_free_addr()
> > +{
> > +   void *p;
> > +   int ret;
> > +   int shm_id = -1;
> > +
> > +   if (probe_key == 0)
> > +           probe_key = getipckey();
> > +
> > +   /* create a shared memory resource with read and write
> > permissions
> > +    * We align this to SHMLBA so we should allocate at least
> > +    * SHMLBA*2 in case SHMLBA > page_size. */
> > +   shm_id = shmget(probe_key, SHMLBA*2, SHM_RW | IPC_CREAT |
> > IPC_EXCL);
> > +   if (shm_id == -1)
> > +           tst_brkm(TBROK, cleanup, "probe: shmget failed");
> > +
> > +   /* Probe an available linear address for attachment */
> > +   p = shmat(shm_id, NULL, 0);
> > +   if (p == (void *)-1)
> > +           tst_brkm(TBROK, cleanup, "probe: shmat failed");
> > +   ret = shmdt((const void *)p);
> > +   if (ret == -1)
> > +           tst_brkm(TBROK, cleanup, "probe: shmdt failed");
> > +
> > +   rm_shm(shm_id);
> > +
> > +   /* some architectures (e.g. parisc) are strange, so better always
> > +    * align to next SHMLBA address. */
> > +   p = (void *)(((unsigned long)(p) + (SHMLBA - 1)) & ~(SHMLBA -
> > 1));
> > +   return p;
> > +}
> > +
> 
> Here, add a new blank line.

I'm not following you here. Do you want 2 blank lines at the end of file?

Regards,
Jan

------------------------------------------------------------------------------
The Windows 8 Center - In partnership with Sourceforge
Your idea - your app - 30 days.
Get started!
http://windows8center.sourceforge.net/
what-html-developers-need-to-know-about-coding-windows-8-metro-style-apps/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to