On 06.12.2006 [17:52:47 +1100], David Gibson wrote:
> On Tue, Dec 05, 2006 at 06:00:23PM -0800, Nishanth Aravamudan wrote:
> > On 05.12.2006 [10:52:03 +1100], David Gibson wrote:
> > > On Mon, Dec 04, 2006 at 03:39:09PM -0800, Nishanth Aravamudan wrote:
> > > > It was noted recently that checking for a hugetlbfs mountpoint in the
> > > > run_tests.sh can lead to a few issues (spaces in /proc/mounts, for
> > > > instance), which are hard to work around. Instead, make use of the
> > > > library's utilities via a small helper program which should provide
> > > > greater safety.
> > > >
> > > > Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>
> >
> > <snip>
> >
> > > >  ALLTESTS = $(foreach DIR,$(OBJDIRS),$(TESTS:%=$(DIR)/%))
> > > > +ALLHELPERS = $(foreach DIR,$(OBJDIRS),$(HELPERS:%=$(DIR)/%))
> > >
> > > Building both 32-bit and 64-bit versions of this is a bit yucky, but I
> > > guess it's not worth the Makefile infrastructure (introducing things
> > > for building "native" objects) to avoid it.
> >
> > Yep, that was my thought. We do have "NATIVEONLY" in the top-level
> > Makefile, which I guess we could introduce in the tests/Makefile, but I
> > think this is simpler for now.
> 
> NATIVEONLY doesn't do the same thing though - it builds *everything*
> for only one arch.

Hrm, true. I see what you're saying now. Yeah, I think doing native
objects only for the helpers would overly complicate the Makefile right
now.

> > > > +$(HELPERS:%=obj32/%): %: %.o obj32/testutils.o
> > > > +       @$(VECHO) LD32 "(helper)" $@
> > > > +       $(CC32) $(LDFLAGS) $(LDFLAGS32) -o $@ $^ $(LDLIBS) -lhugetlbfs
> > > > +
> > > > +$(HELPERS:%=obj64/%): %: %.o obj64/testutils.o
> > > > +       @$(VECHO) LD64 "(helper)" $@
> > > > +       $(CC64) $(LDFLAGS) $(LDFLAGS64) -o $@ $^ $(LDLIBS) -lhugetlbfs
> > > > +
> > >
> > > These don't actually need testutils.o
> >
> > Yep, fixed.
> >
> > <snip>
> >
> > > > +       if (hugetlbfs_test_path(dir) == 1) {
> > > > +               printf("%s\n", dir);
> > > > +               return 0;
> > > > +       }
> > >
> > > This test is redundant, hugetlbfs_find_path() already checks that the
> > > path is hugetlbfs.
> >
> > Yep, fixed.
> >
> > Update patch below. I'll commit it if it looks ok to you.
> >
> > It was noted recently that checking for a hugetlbfs mountpoint in the
> > run_tests.sh can lead to a few issues (spaces in /proc/mounts, for
> > instance), which are hard to work around. Instead, make use of the
> > library's utilities via a small helper program which should provide
> > greater safety.
> 
> Looks ok to me.
> 
> > Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>
> Signed-off-by: David Gibson <[EMAIL PROTECTED]>

Thanks, I've applied this and the linkshare cleanup and asked Adam to
pull. He's out today, so ozlabs should get updated tomorrow.

Thanks,
Nish

-- 
Nishanth Aravamudan <[EMAIL PROTECTED]>
IBM Linux Technology Center

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Libhugetlbfs-devel mailing list
Libhugetlbfs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel

Reply via email to