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] [PATCH v2] syscalls/fanotify08: add sanity check for FAN_CLOEXEC
Date: Fri, 11 Aug 2017 10:41:39 +0200	[thread overview]
Message-ID: <20170811084138.GA3341@rei.lan> (raw)
In-Reply-To: <1502421199-6364-1-git-send-email-xzhou@redhat.com>

Hi!
> +#define _GNU_SOURCE
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <sys/syscall.h>
> +#include "tst_test.h"
> +#include "fanotify.h"
> +
> +#if defined(HAVE_SYS_FANOTIFY_H)
> +#include <sys/fanotify.h>
> +
> +static void cleanup(void);

This cleanup definition is completely useless here as the test structure
is defined at the end of the code.

> +static void test01(int flag);
> +static int fd_notify;
> +
> +void test_coe(void)
> +{
> +	test01(FAN_CLOEXEC);
> +	test01(0);
> +}

Can we turn this into two separate testcases?

> +static void test01(int flag)
> +{
> +	int coe;
> +
> +	fd_notify = fanotify_init(FAN_CLASS_NOTIF|flag, O_RDONLY);
> +
> +	if (fd_notify < 0) {
> +
> +		if (errno == ENOSYS) {
> +			tst_brk(TCONF,
> +				"fanotify is not configured in this kernel.");
> +		} else {
> +			tst_brk(TBROK | TERRNO, "fanotify_init failed");
> +		}
> +	}
> +
> +	coe = SAFE_FCNTL(fd_notify, F_GETFD);
> +
> +	if (coe & FD_CLOEXEC) {
> +		if (flag == 0)
> +			tst_res(TFAIL, "set close-on-exit");
> +		else
> +			tst_res(TPASS, "set close-on-exit");
> +	} else {
> +		if (flag == 0)
> +			tst_res(TPASS, "not set close-on-exit");
> +		else
> +			tst_res(TFAIL, "not set close-on-exit");
> +	}

I would mildly prefer if we split this test functions into two one for
FAN_CLOEXEC and one for case without the flag. All we need is to put the
part that checks the fanotify_init() return value into a separate
function and ideally we should put it into a header so that it could be
reused in all fanotify testcases, but unfortunately we would have to
convert the rest of the fanotify tests to the new library first...

> +	SAFE_CLOSE(fd_notify);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd_notify > 0)
> +		SAFE_CLOSE(fd_notify);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = test_coe,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +};
> +
> +#else
> +	TST_TEST_TCONF("system doesn't have required fanotify support");
> +#endif
> -- 
> 1.8.3.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2017-08-11  8:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-02  9:24 [LTP] [PATCH] syscalls/fanotify08: add sanity check for FAN_CLOEXEC Xiong Zhou
2017-06-05 14:03 ` Cyril Hrubis
2017-08-11  3:13   ` [LTP] [PATCH v2] " Xiong Zhou
2017-08-11  8:41     ` Cyril Hrubis [this message]
2017-08-14 14:01       ` [LTP] [PATCH v3 0/3] add SAFE_FANOTIFY_INIT and FAN_CLOEXEC cases Xiong Zhou
2017-08-14 14:01         ` [LTP] [PATCH v3 1/3] lib: add helper SAFE_FANOTIFY_INIT Xiong Zhou
2017-08-15 13:23           ` Cyril Hrubis
2017-08-14 14:01         ` [LTP] [PATCH v3 2/3] fanotify: rewrite old cases with new lib Xiong Zhou
2017-08-15 13:33           ` Cyril Hrubis
2017-08-14 14:01         ` [LTP] [PATCH v3 3/3] syscalls/fanotify0[89]: add sanity check for FAN_CLOEXEC Xiong Zhou
2017-08-15 13:36           ` Cyril Hrubis
2017-08-16  7:15             ` [LTP] [PATCH v4 0/3] add SAFE_FANOTIFY_INIT and FAN_CLOEXEC check Xiong Zhou
2017-08-16  7:15               ` [LTP] [PATCH v4 1/3] lib: add helper SAFE_FANOTIFY_INIT Xiong Zhou
2017-08-18 10:22                 ` Cyril Hrubis
2017-08-21  3:12                   ` Xiong Zhou
2017-08-21 11:45                     ` Cyril Hrubis
2017-08-16  7:15               ` [LTP] [PATCH v4 2/3] fanotify: rewrite old cases with new lib Xiong Zhou
2017-08-18 10:24                 ` Cyril Hrubis
2017-08-16  7:15               ` [LTP] [PATCH v4 3/3] syscalls/fanotify08: add sanity check for FAN_CLOEXEC Xiong Zhou
2017-08-18 10:25                 ` Cyril Hrubis

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=20170811084138.GA3341@rei.lan \
    --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