From: Jozef Pupava <jpupava@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] Update syscalls/fsync02 to new API
Date: Thu, 26 Mar 2020 11:15:31 +0100 [thread overview]
Message-ID: <20200326111531.163fab48@daedruan> (raw)
In-Reply-To: <20200326083759.GA29830@dell5510>
Hi Petr,
I will add return for better readability in second patch.
Jozef
On Thu, 26 Mar 2020 09:37:59 +0100
Petr Vorel <pvorel@suse.cz> wrote:
> Hi Jozef,
>
> LGTM, thanks for your patch.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> > + if (time_end == -1) {
> > + tst_res(TFAIL | TTERRNO, "getting end time
> > failed");
> > + } else if (TST_RET == -1) {
> > + tst_res(TFAIL | TTERRNO, "fsync failed");
> > + } else if (TST_RET != 0) {
> > + tst_res(TFAIL | TTERRNO,
> > + "fsync failed with unexpected return value");
> > + } else if (time_end < time_start) {
> > + tst_res(TFAIL,
> > + "timer broken end %ld < start %ld",
> > + time_end, time_start);
> > + } else if ((time_delta =
> > + difftime(time_end, time_start)) > TIME_LIMIT) {
> > + tst_res(TFAIL,
> > + "fsync took too long: %lf seconds; "
> > + "max_block: %d; data_blocks: %d",
> > + time_delta, max_block, data_blocks);
> > + } else {
> > + tst_res(TPASS,
> > + "fsync succeeded in an acceptable amount of time");
> > + }
>
> nit: note Cyril prefers due better readability return from function
> instead of having too much else if. So something like this might be
> more readable. But up to you.
>
> if (time_end == -1) {
> tst_res(TFAIL | TTERRNO, "getting end time failed");
> return;
> }
>
> if (TST_RET == -1) {
> tst_res(TFAIL | TTERRNO, "fsync failed");
> return;
> }
>
> if (TST_RET != 0) {
> tst_res(TFAIL | TTERRNO, "fsync failed with
> unexpected return value"); return;
> }
>
> if (time_end < time_start) {
> tst_res(TFAIL, "timer broken end %ld < start %ld",
> time_end, time_start);
> return;
> }
>
> if ((time_delta = difftime(time_end, time_start)) >
> TIME_LIMIT) { tst_res(TFAIL,
> "fsync took too long: %lf seconds; max_block: %d;
> data_blocks: %d", time_delta, max_block, data_blocks);
> return;
> }
>
> tst_res(TPASS, "fsync succeeded in an acceptable amount of
> time");
>
> Kind regards,
> Petr
>
next prev parent reply other threads:[~2020-03-26 10:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 13:38 [LTP] [PATCH 1/2] Update syscalls/fsync02 to new API Jozef Pupava
2020-03-26 8:37 ` Petr Vorel
2020-03-26 10:15 ` Jozef Pupava [this message]
2020-03-26 9:38 ` Martin Doucha
2020-03-26 10:10 ` Jozef Pupava
2020-03-26 10:13 ` Petr Vorel
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=20200326111531.163fab48@daedruan \
--to=jpupava@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