public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [RFC PATCH] Add library support for /proc/sys/kernel/tainted
Date: Mon, 22 Jan 2018 16:39:42 +0100	[thread overview]
Message-ID: <20180122153942.GA17451@rei> (raw)
In-Reply-To: <20180122131218.26407-1-mmoese@suse.de>

Hi!
> +/*
> + * check if kernel is recent enough to support the tainted-flags that
> + * allow us to check if the kernel issued a warning or died
> + *
> + * returns 1 if supported, 0 otherwise
> + */
> +int tst_taint_supported(void);
> +
> +/*
> + * read /proc/sys/kernel/tainted and return the value
> + * this function allows to be called even when the relevant flags are
> + * unsupported.
> + *
> + * returns bitmask of taint flag, or (unsigned int) -1 on read error.
> + */
> +unsigned int tst_taint_read(void);
> +
> +/*
> + * a small helper function to check if flags specified by mask are set
> + *
> + * returns 1 if at least one of the flags is set, 0 otherwise
> + */
> +int tst_taint_check_mask(unsigned int taint, unsigned int mask);
> +
> +#endif /* TST_TAINTED_H__ */

This patch lacks information on how this API is supposed to be used
which is quite important for the review. What I mean is that, while
important, the documentation for the function parameters and return
values is not enough. What should be included is some sketch code of
this is supposed to be used from an actual test source.

> diff --git a/lib/tst_taint.c b/lib/tst_taint.c
> new file mode 100644
> index 000000000..38bf355ec
> --- /dev/null
> +++ b/lib/tst_taint.c
> @@ -0,0 +1,69 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/utsname.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include <linux/version.h>
> +
> +#include "tst_test.h"
> +#include "tst_taint.h"
> +#include "tst_safe_stdio.h"
> +
> +#define TAINT_FILE "/proc/sys/kernel/tainted"
> +
> +static int taint_supported = -1;
> +
> +int tst_taint_supported(void)
> +{
> +	struct utsname uname_data;
> +	int maj, min, patch;
> +
> +	if (uname(&uname_data) == -1)
> +		return -1;
> +
> +	if (sscanf(uname_data.release, "%d.%d.%d-", &maj, &min, &patch) != 3)
> +		fprintf(stderr, "error parsing uname information\n");
> +
> +	if ((maj < 2) || ((maj == 2) && (min < 26))) {
> +		fprintf(stderr, "Kernel version prior to 2.6.26 detected.\n");
> +		fprintf(stderr, "please check kernel output manually\n");

These messages should go out via tst_res().

> +		taint_supported = 0;
> +	} else
> +		taint_supported = 1;

We have tst_kvercmp() exactly for this purpose, see include/tst_kvercmp.h

> +	return taint_supported;
> +}
> +
> +unsigned int tst_taint_read(void)
> +{
> +	int fd;
> +	unsigned int val;
> +	char buffer[11];
> +
> +	if (taint_supported == -1)
> +		tst_taint_supported();

So this will only cause warning to be printed into the stderr, then we
go ahead and read the file? Shouldn't we just tst_brk(TCONF, "") at this
point?

> +	buffer[10] = 0;
> +
> +	fd = SAFE_OPEN(TAINT_FILE, O_RDONLY);
> +
> +	val = read(fd, buffer, 10);
> +	SAFE_CLOSE(fd);
> +	if (val == -1) {
> +		fprintf(stderr, "unable to read %s\n", TAINT_FILE);
> +		return (unsigned int) -1;
> +	}
> +	val = safe_atoi(buffer);

What about using SAFE_FILE_SCANF() instead?

> +	return val;
> +}
> +
> +int tst_taint_check_mask(unsigned int taint, unsigned int mask)
> +{
> +	if ((taint & mask) != 0)
> +		return 1;
> +	return 0;
> +}

Hmm, should this be combined with the read function?

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2018-01-22 15:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-22 13:12 [LTP] [RFC PATCH] Add library support for /proc/sys/kernel/tainted Michael Moese
2018-01-22 15:39 ` Cyril Hrubis [this message]
2018-01-23  9:16   ` Michael Moese

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180122153942.GA17451@rei \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox