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?
Also, unless the test needs the filename -- we should be using
mkstemp as [g]libc suggests.
Cheers,
-Garrett
-------------------------------------------------------------------------
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
