On Fri, Apr 8, 2011 at 8:23 PM, Han Pingtian <[email protected]> wrote:
> I have updated the patch based on your suggestions. Please review.
>
> Thanks.
An inline diff would have been nice. Anyhow..
...
+#include "test.h"
+#include "usctest.h"
+#include "config.h"
+
+char *TCID = "thp01";
+int TST_TOTAL = 1;
+
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <sys/types.h>
+#include <sys/wait.h>
Please read the style guide and the example code I've provided in the
style guide. This doesn't conform to those examples.
+int main(int argc, char **argv)
+{
+ int i, lc, st;
+ pid_t pid;
+ char *msg;
+ char *c[257];
+ char cc[32*4096];
+ struct rlimit rl = {
+ .rlim_cur = RLIM_INFINITY,
+ .rlim_max = RLIM_INFINITY,
+ };
+
+ if ((msg = parse_opts(argc, argv, NULL, NULL)) != NULL)
+ tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
+
+ for (lc = 0; TEST_LOOPING(lc); lc++) {
+ switch (pid = fork()) {
+ case -1:
Unnecessary indentation.
+ tst_brkm(TBROK|TERRNO, NULL, "fork");
+ case 0:
+ memset(cc, 'c', 32*4096-1);
+ cc[32*4096-1] = '\0';
Make the magic number (32*4096-1) a number. BTW -- did you derive this
from a pagesize or something? If so, you should really use the sysconf
function to derive _SC_PAGESIZE.
+ for (i=0;i<256;i++)
[Lack of] whitespace.
+ c[i] = cc;
+ if (setrlimit(RLIMIT_STACK, &rl) == -1) {
+ perror("setrlimit");
+ exit(1);
+ }
+ if (execve("/bin/true", c, c) == -1) {
+ perror("execve");
+ exit(2);
+ }
So, this isn't supposed to exit I assume? Seems kind of funky (i.e.
would run out of processes).
+ default:
+ if (waitpid(pid, &st, 0) == -1)
+ tst_brkm(TBROK|TERRNO, NULL, "waitpid");
+
+ if (!WIFEXITED(st))
+ tst_brkm(TBROK, NULL, "child exits
abnormally");
*exited.
+ if (WEXITSTATUS(st) == 2)
+ tst_brkm(TBROK, NULL, "Do you have
/bin/true installed?");
Add a check at the beginning of the test to ensure (via stat) that
/bin/true exists. That way you can skip this check.
+ if (WEXITSTATUS(st) != 0)
+ tst_brkm(TBROK, NULL, "chaild exits
with non-zero value");
You didn't do a proper exit(0). How is this possible (unless the
forked child runs to completion and exits the loop first which just
seems like a bad idea because you're executing tst_exit() at the
bottom)?
+ tst_resm(TPASS, "thp01 pass");
How do you know it passes from just one run when it could cascade over
several iterations? My gut feeling is that this really should be moved
somewhere else.
+ }
+ }
+
+ tst_exit();
Indentation is off.
+}
------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
