Hi!
> +/******************************************************************************/
> +/*
> */
> +/* Copyright (c) International Business Machines Corp., 2008
> */
> +/*
> */
> +/* This program is free software; you can redistribute it and/or modify
> */
> +/* it under the terms of the GNU General Public License as published by
> */
> +/* the Free Software Foundation; either version 2 of the License, or
> */
> +/* (at your option) any later version.
> */
> +/*
> */
> +/* This program is distributed in the hope that it will be useful,
> */
> +/* but WITHOUT ANY WARRANTY; without even the implied warranty of
> */
> +/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> */
> +/* the GNU General Public License for more details.
> */
> +/*
> */
> +/* You should have received a copy of the GNU General Public License
> */
> +/* along with this program.
> */
> +/*
> */
> +/******************************************************************************/
> +
> +#include <stdio.h>
> +#include "config.h"
> +#include "test.h"
> +#if HAVE_SYS_CAPABILITY_H
> +#include <linux/types.h>
> +#include <sys/capability.h>
> +#endif
> +
> +char *TCID = "filecaps01";
^
Since TCID is set to filecaps01 it would make more
sense to rename the file to filecaps01.c so that the
filename and TCID match.
> +int TST_TOTAL = 1;
> +
> +static void cleanup(void)
> +{
> + tst_rmdir();
> +}
> +
> +static void setup(void)
> +{
> + tst_tmpdir();
> +}
Looking at this test in particular it doesn't need temporary directory,
so we can omit this.
> +int main(int argc, char *argv[])
> +{
> +#ifdef HAVE_LIBCAP
> + int lc;
> + cap_t caps, caps2;
> + int ret;
> +#endif
> + tst_parse_opts(argc, argv, NULL, NULL);
> + setup();
> +
> +#ifdef HAVE_LIBCAP
> + for (lc = 0; TEST_LOOPING(lc); lc++) {
> + caps = cap_from_text("cap_setpcap+ep");
> + caps2 = cap_from_text("cap_setpcap+ep");
> + ret = cap_set_proc(caps);
> + if (ret != 0)
> + tst_resm(TFAIL, "cap_set_proc failure");
^
This should be tst_brkm(TBROK, ...); so that
we exit the test if the preparation failed.
> + ret = cap_compare(caps, caps2);
This seems really strange, we compile two identical capabilities from
string and then compare them for equality. Shouldn't the code rather:
* prepare capability from string
* set the capability
* get the capability back
* compare the compiled value with the one we get back
Otherwise this test does not make much sense to me.
> + if (ret != 0)
> + tst_resm(TFAIL, "Caps were not the same.");
> +
> + cap_free(caps);
> + cap_free(caps2);
> + tst_resm(TPASS, "Caps were the same.");
^
You must not print TPASS if the test
> + }
> +#else
> + tst_resm(TFAIL, "System doesn't support full POSIX capabilities.");
^
This should be tst_brkm(TCONF, ...);
Usually testcases tend to have separate main()
function for the case that some functionality was
missing upon compilation rather than spreading
#ifdefs around the code. I.e.
#ifdef HAVE_FOO
int main(int argc, char *argv[])
{
/* do the test here */
...
}
#else
int main(void)
{
tst_brkm(TCONF, "...");
}
#endif /* HAVE_FOO */
> +#endif
> + cleanup();
> + tst_exit();
> +}
--
Cyril Hrubis
chru...@suse.cz
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
