On Monday 20 October 2008 10:39:45 Garrett Cooper wrote:
> On Oct 19, 2008, at 11:35 PM, Subrata Modak wrote:
>
> >
> > On Fri, 2008-10-17 at 16:04 +0200, Daniel Gollub wrote:
> >> Hi CAI,
> >>
> >> On Friday 17 October 2008 12:11:35 CAI Qian wrote:
> >>> diff -Nur ltp/testcases/kernel/containers/pidns/pidns03.c
> >>> ltp-new/testcases/kernel/containers/pidns/pidns03.c
> >>> --- ltp/testcases/kernel/containers/pidns/pidns03.c 2008-10-15
> >>> 21:39:45.000000000 +0800
> >>> +++ ltp-new/testcases/kernel/containers/pidns/pidns03.c 2008-10-16
> >>> 14:48:52.635233426 +0800
> >>> @@ -80,7 +80,7 @@
> >>> ppid = getpid();
> >>>
> >>> /* Create a Container and execute to test the functionality
> >>> */
> >>> - ret = do_clone_unshare_test(T_CLONE, CLONE_NEWPID|
> >>> CLONE_NEWNS, child_fn, ppid);
> >>> + ret = do_clone_unshare_test(T_CLONE, CLONE_NEWPID|
> >>> CLONE_NEWNS, child_fn, &ppid);
> >>>
> >>> /* check return code */
> >>> if (ret == -1) {
> >>> @@ -116,7 +116,7 @@
> >>> */
> >>>
> >>> int
> >>> -child_fn(pid_t Ppid)
> >>> +child_fn(pid_t *Ppid)
> >>> {
> >>> char dirnam[50];
> >>> DIR *d;
> >>> @@ -129,19 +129,19 @@
> >>> "\t\t\t\tParent namespace pid = %d,"
> >>> "container parent pid = %d,"
> >>> "and container pid = %d\n",
> >>> - Ppid, parent_pid, cloned_pid);
> >>> + *Ppid, parent_pid, cloned_pid);
> >>>
> >>> /* do any /proc setup which winds up being necessary. */
> >>> if (mount("proc", "/proc", "proc", 0, NULL) < 0)
> >>> tst_resm(TFAIL, "mount failed : \n");
> >>>
> >>> /* Check for the parent pid is existing still? */
> >>> - sprintf(dirnam, "/proc/%d", Ppid);
> >>> + sprintf(dirnam, "/proc/%d", *Ppid);
> >>>
> >>> d = opendir(dirnam);
> >>> if (!d) {
> >>> tst_resm(TPASS, \
> >>> - "Got the proc file directory created by parent ns
> >>> %d\n", Ppid);
> >>> + "Got the proc file directory created by parent ns
> >>> %d\n", *Ppid);
> >>> umount("/proc");
> >>> } else {
> >>> tst_resm(TFAIL, "Failed to open /proc directory \n");
> >>
> >> Reviewed-by: Daniel Gollub <[EMAIL PROTECTED]>
> >>
> >> I can confirm this diff-hunk. I had quite similar patch in my queue.
> >> Without this one i get a serious compiler warning in our Build System
> >> which requires to get fixed and doesn't let the build of ltp pass.
> >>
> >> Nice work! I try to review some more...
> >
> > Thanks Cai for fixing all those warnings. Almost all are gone. Thanks
> > Daniel for providing comments. This has been checked in. Can you
> > please
> > also look into the residual few below:
> >
> > make[3]: Entering directory
> > `/SEPTEMBER_2008_RELEASE/ltp-full-20080930/testcases/ballista/
> > ballista'
> > g++ -Wno-deprecated -Wall -w -DB_SELFHOST callGen.cpp -o callGen
> > /tmp/ccImvj9h.o: In function
> > `__static_initialization_and_destruction_0(int, int)':
> > callGen.cpp:(.text+0x53): warning: the use of `tempnam' is dangerous,
> > better use `mkstemp'
> > g++ -Wno-deprecated -Wall -w -DB_SELFHOST callGen_standAlone.cpp -o
> > callGen_standAlone
> > /tmp/cc2f4HI9.o: In function
> > `__static_initialization_and_destruction_0(int, int)':
> > callGen_standAlone.cpp:(.text+0x53): warning: the use of `tempnam' is
> > dangerous, better use `mkstemp'
> > g++ -Wno-deprecated -Wall -w -DB_SELFHOST genCodeCreator.cpp -o
> > genCodeCreator
> >
> > Regards--
> > Subrata
> >
> >>
> >> best regards,
> >> Daniel
>
> Daniel,
> I agree with the first change, but I disagree with the subsequent
> changes as you don't have a need for dereferencing a pid_t object
> within the functions as the value referred to by pPid is constant;
> plus on 64-bit platforms the change is wasting an extra 4-bytes (just
> to nitpick -- I admit), in addition to the dereference operation per
> instance.
> So getting back to the original issue -- what was the original -
> Werror reported item?
make[1]: Entering directory
`/home/dgollub/home:dgollub/ltp/ltp-full-20081017/testcases/kernel/containers/pidns'
cc -O2 -Wall -Wall -I../../../../include -I../libclone pidns03.c
-L../../../../lib -L../libclone ../libclone/libclone.a -lltp -o pidns03
pidns03.c: In function ‘main’:
pidns03.c:83: warning: passing argument 4 of ‘do_clone_unshare_test’ makes
pointer from integer without a cast
make[1]: Leaving directory
`/home/dgollub/home:dgollub/ltp/ltp-full-20081017/testcases/kernel/containers/pidns'
Was the original compiler warning - which is related to the first diff-hunk.
> Also, unless the test needs the filename -- we should be using
> mkstemp as [g]libc suggests.
> Cheers,
> -Garrett
>
best regards,
Daniel
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
