> TESTDIR isn't publicly visible anymore because people really shouldn't
> be mucking around with that value, or querying it if they haven't
> called tst_tmpdir (which is actually what you did in this test!).
I am sorry about that. That is why I had sent a follow-up patch to address
it,
http://marc.info/?l=ltp-list&m=129379291515012&w=2
> > Again, what issues are your facing with this code?
> >> + 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);
> >> - }
> > As you can read from the help(), the test can allocate hugepages
> > either
> > from shmget() or mmap(). In case of shmget(), a global variable
> > shmid with
> > the default value -1 will be change to 1 via command-line,
>
> Ok, I must have missed that part. Sorry...
>
> > static option_t options[] = {
> > { "s", &opt_sysfs, NULL},
> > { "m", &shmid, NULL},
> > { "a:", &opt_alloc, &opt_allocstr},
> > { NULL, NULL, NULL}
> > };
> >
> > Therefore, there is a need to call shmat(). That is why it only call
> > it
> > when shmid != -1. In case of mmap(), there is no need to call
> > shmat().
> >> + 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.");
> > If you are going to change the syntax here, there are many other
> > places
> > also need to changed to be consistent like,
> > "check sysfs."
> > "check /proc/meminfo before allocation."
> > etc
>
> Agreed. It was just an example, and the point was to clearly state
> with proper verb tenses so that people could get a clear idea of what
> the test was doing instead of having to read the code to decypher the
> intent of the test writer. Granted, I've gotten lazy and stopped
> fixing some of these items in later commits because of all of the LOCs
> I've made, but it needs to be fixed later because the grammar and
> spelling used by test writers is less than desirable, which makes it
> hard for folks trying to understand what the intention is behind these
> tests.
OK.
Thanks.
CAI Qian
------------------------------------------------------------------------------
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
