Looking over the code, there are a number of flawed assumptions in
terms of how things are executed and it seems that the code is overly
complex. Before I go and do something rash though, I wanted others to
look at the code (in particular the areas I'm touching) and see
whether or not I'm just blowing something out my rear.
    As-is the code doesn't compile due to warnings.
    This is a WIP patch (please ignore the whitespace and style to a
small degree as I know gmail will fubar the formatting).
Thanks,
-Garrett

diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
index 8893f84..2ea1363 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
@@ -97,7 +97,7 @@ static option_t options[] = {
        { NULL, NULL,           NULL}
 };
 static void setup(void);
-static void cleanup(void) LTP_ATTRIBUTE_NORETURN;
+static void cleanup(void);
 static void overcommit(void);
 static void write_bytes(void *addr);
 static void read_bytes(void *addr);
@@ -135,6 +135,7 @@ int main(int argc, char *argv[])
                overcommit();
        }
        cleanup();
+       tst_exit();
 }

 static void overcommit(void)
@@ -154,8 +155,15 @@ static void overcommit(void)
                if (shmid == -1)
                        tst_brkm(TBROK|TERRNO, cleanup, "shmget");
        } else {
-               snprintf(s, BUFSIZ, "%s/hugemmap05/file", get_tmp_tstdir());
-               fd = open(s, O_CREAT | O_RDWR, 0755);
+               char *tmp_tstdir;
+
+               tmp_tstdir = get_tst_tmpdir();
+               if (tmp_tstdir == NULL)
+                       tst_brkm(TBROK|TERRNO, cleanup, "get_tmp_tstdir 
failed");
+               snprintf(s, BUFSIZ, "%s/hugemmap05/file", tmp_tstdir);
+               free(tmp_tstdir);
+
+               fd = open(s, O_CREAT|O_RDWR, 0755);
                if (fd == -1)
                        tst_brkm(TBROK|TERRNO, cleanup, "open");
                addr = mmap(ADDR, (long)(length * MB), PROTECTION, FLAGS, fd,
@@ -195,19 +203,14 @@ static void overcommit(void)
                        return;
                fclose(fp);
        }
-       if (shmid != -1) {
-               tst_resm(TINFO, "shmid: 0x%x", shmid);
-               shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS);
-               if (shmaddr == (void *)-1)
-                       tst_brkm(TBROK|TERRNO, cleanup, "shmat");
-               write_bytes(shmaddr);
-               read_bytes(shmaddr);
-        } else {
-               write_bytes(addr);
-               read_bytes(addr);
-       }
+       tst_resm(TINFO, "shmid: 0x%x", shmid);
+       shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS);
+       if (shmaddr == (void *)-1)
+               tst_brkm(TBROK|TERRNO, cleanup, "shmat");
+       write_bytes(shmaddr);
+       read_bytes(shmaddr);
        if (opt_sysfs) {
-               tst_resm(TINFO, "check sysfs.");
+               tst_resm(TINFO, "checking sysfs.");
                if (checksys(_PATH_SYS_2M_HUGE, "HugePages_Total",
                                length / 2) != 0)
                        return;
@@ -221,7 +224,7 @@ static void overcommit(void)
                        != 0)
                        return;
        } else {
-               tst_resm(TINFO, "check /proc/meminfo.");
+               tst_resm(TINFO, "checking /proc/meminfo.");
                fp = fopen(_PATH_MEMINFO, "r");
                if (fp == NULL)
                        tst_brkm(TBROK|TERRNO, cleanup, "fopen");
@@ -236,18 +239,17 @@ static void overcommit(void)
                        return;
                fclose(fp);
        }
-       if (shmid != -1) {
-               if (shmdt(shmaddr) != 0)
-                       tst_brkm(TBROK|TERRNO, cleanup, "shmdt");
-       } else {
-               munmap(addr, (long)(length * MB));
-               close(fd);
-               unlink(s);
-       }
+       shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS);
+       if (shmaddr == (void *)-1)
+               tst_brkm(TBROK|TERRNO, cleanup, "shmat failed");
+       if (shmdt(shmaddr) != 0)
+               tst_brkm(TBROK|TERRNO, cleanup, "shmdt failed");
 }

 static void cleanup(void)
 {
+       char *tmp_tstdir;
+       void *shmaddr;
        int fd;

        TEST_CLEANUP;
@@ -262,38 +264,49 @@ static void cleanup(void)
        fd = open(path, O_WRONLY);
        if (fd == -1)
                tst_resm(TWARN|TERRNO, "open");
-       tst_resm(TINFO, "restore nr_hugepages to %s.", nr_hugepages);
-       if (write(fd, nr_hugepages,
-                       strlen(nr_hugepages)) != strlen(nr_hugepages))
+       tst_resm(TINFO, "restoring nr_hugepages to %s", nr_hugepages);
+       if (write(fd, nr_hugepages, strlen(nr_hugepages)) !=
+           strlen(nr_hugepages))
                tst_resm(TWARN|TERRNO, "write");
        close(fd);

        fd = open(pathover, O_WRONLY);
        if (fd == -1)
                tst_resm(TWARN|TERRNO, "open");
-       tst_resm(TINFO, "restore nr_overcommit_hugepages to %s.",
+       tst_resm(TINFO, "restoring nr_overcommit_hugepages to %s.",
                nr_overcommit_hugepages);
        if (write(fd, nr_overcommit_hugepages, strlen(nr_overcommit_hugepages))
                != strlen(nr_overcommit_hugepages))
                tst_resm(TWARN|TERRNO, "write");
        close(fd);

-       snprintf(buf, BUFSIZ, "%s/hugemmap05", get_tst_tmpdir());
+       tmp_tstdir = get_tst_tmpdir();
+       if (tmp_tstdir == NULL)
+               tst_brkm(TBROK|TERRNO, NULL, "tmp_tstdir failed");
+       snprintf(buf, BUFSIZ, "%s/hugemmap05", tmp_tstdir);
+       free(tmp_tstdir);
+       shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS);
+       if (shmaddr == (void *)-1)
+               tst_brkm(TBROK|TERRNO, cleanup, "shmat");
+       else if (shmdt(shmaddr) == -1)
+               tst_resm(TBROK|TERRNO, "shmdt failed");
+       else if (shmctl(shmid, IPC_RMID, NULL) == -1)
+               tst_resm(TBROK|TERRNO,
+                   "shmctl(..., IPC_RMID, ...) failed");
        if (umount(buf) == -1)
                tst_resm(TWARN|TERRNO, "umount");
-       if (shmid != -1) {
-               tst_resm(TINFO|TERRNO, "shmdt");
-               shmctl(shmid, IPC_RMID, NULL);
-       }
+       tst_rmdir();

 }

 static void setup(void)
 {
        FILE *fp;
+       char *tmp_tstdir;
        int fd;

        tst_sig(FORK, DEF_HANDLER, cleanup);
+       tst_tmpdir();
        TEST_PAUSE;
        if (shmid != -1) {
                fp = fopen(_PATH_SHMMAX, "r");
@@ -332,7 +345,7 @@ static void setup(void)
                tst_brkm(TBROK|TERRNO, cleanup, "write");
        if (lseek(fd, 0, SEEK_SET) == -1)
                tst_brkm(TBROK|TERRNO, cleanup, "lseek");
-       snprintf(buf, BUFSIZ, "%ld", size);
+       snprintf(buf, BUFSIZ, "%zd", size);
        if (write(fd, buf, strlen(buf)) != strlen(buf))
                tst_brkm(TBROK|TERRNO, cleanup,
                        "failed to change nr_hugepages.");
@@ -356,13 +369,18 @@ static void setup(void)
                tst_brkm(TBROK|TERRNO, cleanup, "write");
        if (lseek(fd, 0, SEEK_SET) == -1)
                tst_brkm(TBROK|TERRNO, cleanup, "lseek");
-       snprintf(buf, BUFSIZ, "%ld", size);
+       snprintf(buf, BUFSIZ, "%zd", size);
        if (write(fd, buf, strlen(buf)) != strlen(buf))
                tst_brkm(TBROK|TERRNO, cleanup,
                        "failed to change nr_hugepages.");
        close(fd);

-       snprintf(buf, BUFSIZ, "%s/hugemmap05", get_tst_tmpdir());
+       tmp_tstdir = get_tst_tmpdir();
+       if (tmp_tstdir == NULL)
+               tst_brkm(TBROK|TERRNO, NULL, "tmp_tstdir failed");
+       snprintf(buf, BUFSIZ, "%s/hugemmap05", tmp_tstdir);
+       free(tmp_tstdir);
+
        if (mkdir(buf, 0700) == -1)
                tst_brkm(TBROK|TERRNO, cleanup, "mkdir");
        if (mount(NULL, buf, "hugetlbfs", 0, NULL) == -1)
@@ -447,4 +465,4 @@ static int checkproc(FILE *fp, char *pattern, int value)
                return 1;
        }
        return 0;
-}
\ No newline at end of file
+}

Attachment: hugemmap05.patch
Description: Binary data

------------------------------------------------------------------------------
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and, 
should the need arise, upgrade to a full multi-node Oracle RAC database 
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to