All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: "Wei,Jiangang" <weijg.fnst@cn.fujitsu.com>
Cc: ltp-list@lists.sourceforge.net
Subject: Re: [LTP] [PATCH 2/5] testcases/kernel/syscalls/fcntl: use TERRNO flag
Date: Mon, 18 May 2015 13:40:42 +0200	[thread overview]
Message-ID: <20150518114042.GC4104@rei> (raw)
In-Reply-To: <1431515986-22287-2-git-send-email-weijg.fnst@cn.fujitsu.com>

Hi!
> diff --git a/testcases/kernel/syscalls/fcntl/fcntl19.c b/testcases/kernel/syscalls/fcntl/fcntl19.c
> index a542cfc..5775684 100644
> --- a/testcases/kernel/syscalls/fcntl/fcntl19.c
> +++ b/testcases/kernel/syscalls/fcntl/fcntl19.c
> @@ -101,12 +101,11 @@ void setup(void)
>  	snprintf(template, PATH_MAX, "fcntl19XXXXXX");
>  
>  	if ((fd = mkstemp(template)) < 0) {
> -		tst_resm(TFAIL, "Couldn't open temp file! errno = %d", errno);
> +		tst_resm(TFAIL|TERRNO, "Couldn't open temp file!");

Technically this is a good change, but the code was broken beforehand.

The correct thing to do here is:

	tst_brkm(TBROK | TERRNO, cleanup, "Couldn't open temp file.");

Otherwise the testcase will attempt to call write() on fd == -1.

Moreover it can also do simple open instead of mkstemp() because the
temporary directory made with tst_tmpdir() is unique itself.

>  	}
>  
>  	if (write(fd, buf, STRINGSIZE) < 0) {
> -		tst_resm(TFAIL, "Couldn't write to temp file! errno = %d",
> -			 errno);
> +		tst_resm(TFAIL|TERRNO, "Couldn't write to temp file!");
>  	}

Here it could be changed to SAFE_WRITE();

>  	memset(&act, 0, sizeof(act));
> @@ -114,7 +113,7 @@ void setup(void)
>  	sigemptyset(&act.sa_mask);
>  	sigaddset(&act.sa_mask, SIGCLD);
>  	if ((sigaction(SIGCLD, &act, NULL)) < 0) {
> -		tst_resm(TFAIL, "SIGCLD signal setup failed, errno: %d", errno);
> +		tst_resm(TFAIL|TERRNO, "SIGCLD signal setup failed!");
>  		fail = 1;
>  	}
>  }
> @@ -203,7 +202,7 @@ void unlock_file(void)
>  	struct flock fl;
>  
>  	if (do_lock(F_SETLK, (short)F_UNLCK, (short)0, 0, 0) < 0) {
> -		tst_resm(TFAIL, "fcntl on file failed, errno =%d", errno);
> +		tst_resm(TFAIL|TERRNO, "fcntl on file failed!");
>  		fail = 1;
>  	}


Hmm, this should be tst_brkm(TBROK ... ); as well as this is not the
test but rather cleanup/preparation for next test.

Basically all cases where some prepartion fails should be TBROK rather
than TFAIL and should also exit the test if needed preparation steps
have failed.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

  reply	other threads:[~2015-05-18 11:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 11:19 [LTP] [PATCH 1/5] lib/safe_file_ops: fix typo Wei,Jiangang
2015-05-13 11:19 ` [LTP] [PATCH 2/5] testcases/kernel/syscalls/fcntl: use TERRNO flag Wei,Jiangang
2015-05-18 11:40   ` Cyril Hrubis [this message]
2015-05-13 11:19 ` [LTP] [PATCH 3/5] testcases/kernel: replace umount() with tst_umount() Wei,Jiangang
2015-05-13 11:19 ` [LTP] [PATCH 4/5] testcases/kernel/fs/doio: free memory Wei,Jiangang
2015-05-18 11:28   ` Cyril Hrubis
2015-05-13 11:19 ` [LTP] [PATCH 5/5] network/nfs/nfsstress: fix printf format Wei,Jiangang
2015-05-18 11:45 ` [LTP] [PATCH 1/5] lib/safe_file_ops: fix typo 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=20150518114042.GC4104@rei \
    --to=chrubis@suse.cz \
    --cc=ltp-list@lists.sourceforge.net \
    --cc=weijg.fnst@cn.fujitsu.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.