* [LTP] [PATCH] quotactl01: replace int comparison with memcmp
@ 2017-01-13 11:43 Jan Stancek
2017-01-16 2:13 ` Xiao Yang
2017-01-16 11:50 ` Cyril Hrubis
0 siblings, 2 replies; 4+ messages in thread
From: Jan Stancek @ 2017-01-13 11:43 UTC (permalink / raw)
To: ltp
Testcase currently uses unsigned long for all returned
and expected values. This doesn't work for some cases,
for example Q_GETFMT returns 32bit number. So if we pass
in unsigned long (8 bytes) on big endian, the comparison
breaks:
quotactl01.c:198: FAIL: quotactl got unexpected info 8589934592, expected 2
2 << 32 == 8589934592
This patch converts parameters to void* and adds size value. After
syscall it passes all to memcmp.
Patch also removes dummy "data" variable and replaces it with
NULL and 0 size parameters.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
testcases/kernel/syscalls/quotactl/quotactl01.c | 99 +++++++++++++++----------
1 file changed, 58 insertions(+), 41 deletions(-)
diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
index 586272af1d0c..fb74a8a6404f 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl01.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
@@ -79,9 +79,8 @@
# define MNTPOINT "mntpoint"
static int mount_flag;
-static unsigned long fmt_id = FMTID;
-static unsigned long test_id;
-static unsigned long data;
+static int32_t fmt_id = FMTID;
+static int test_id;
static struct dqblk set_dq = {
.dqb_bsoftlimit = 100,
.dqb_valid = QIF_BLIMITS
@@ -93,53 +92,70 @@ static struct dqinfo set_qf = {
.dqi_valid = IIF_BGRACE
};
static struct dqinfo res_qf;
-static unsigned long fmt_buf[32];
+static int32_t fmt_buf;
# endif
static struct tcase {
int cmd;
- unsigned long *id;
+ int *id;
void *addr;
- unsigned long *set_data;
- unsigned long *res_data;
+ void *set_data;
+ void *res_data;
+ int sz;
char *des;
} tcases[] = {
- {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, USRPATH, &data, &data,
- "turn on quota for user"},
- {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dq, &data, &data,
- "set disk quota limit for user"},
- {QCMD(Q_GETQUOTA, USRQUOTA), &test_id, &res_dq, &set_dq.dqb_bsoftlimit,
- &res_dq.dqb_bsoftlimit, "get disk quota limit for user"},
+ {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, USRPATH,
+ NULL, NULL, 0, "turn on quota for user"},
+
+ {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dq,
+ NULL, NULL, 0, "set disk quota limit for user"},
+
+ {QCMD(Q_GETQUOTA, USRQUOTA), &test_id, &res_dq,
+ &set_dq.dqb_bsoftlimit, &res_dq.dqb_bsoftlimit,
+ sizeof(res_dq.dqb_bsoftlimit), "get disk quota limit for user"},
# if defined(HAVE_QUOTAV2)
- {QCMD(Q_SETINFO, USRQUOTA), &test_id, &set_qf, &data, &data,
- "set information about quotafile for user"},
- {QCMD(Q_GETINFO, USRQUOTA), &test_id, &res_qf, &set_qf.dqi_bgrace,
- &res_qf.dqi_bgrace, "get information about quotafile for user"},
- {QCMD(Q_GETFMT, USRQUOTA), &test_id, fmt_buf, &fmt_id, fmt_buf,
+ {QCMD(Q_SETINFO, USRQUOTA), &test_id, &set_qf,
+ NULL, NULL, 0, "set information about quotafile for user"},
+
+ {QCMD(Q_GETINFO, USRQUOTA), &test_id, &res_qf,
+ &set_qf.dqi_bgrace, &res_qf.dqi_bgrace, sizeof(res_qf.dqi_bgrace),
+ "get information about quotafile for user"},
+
+ {QCMD(Q_GETFMT, USRQUOTA), &test_id, &fmt_buf,
+ &fmt_id, &fmt_buf, sizeof(fmt_buf),
"get quota format for user"},
# endif
- {QCMD(Q_SYNC, USRQUOTA), &test_id, &res_dq, &data, &data,
- "update quota usages for user"},
- {QCMD(Q_QUOTAOFF, USRQUOTA), &test_id, USRPATH, &data, &data,
- "turn off quota for user"},
- {QCMD(Q_QUOTAON, GRPQUOTA), &fmt_id, GRPPATH, &data, &data,
- "turn on quota for group"},
- {QCMD(Q_SETQUOTA, GRPQUOTA), &test_id, &set_dq, &data, &data,
- "set disk quota limit for group"},
+ {QCMD(Q_SYNC, USRQUOTA), &test_id, &res_dq,
+ NULL, NULL, 0, "update quota usages for user"},
+
+ {QCMD(Q_QUOTAOFF, USRQUOTA), &test_id, USRPATH,
+ NULL, NULL, 0, "turn off quota for user"},
+
+ {QCMD(Q_QUOTAON, GRPQUOTA), &fmt_id, GRPPATH,
+ NULL, NULL, 0, "turn on quota for group"},
+
+ {QCMD(Q_SETQUOTA, GRPQUOTA), &test_id, &set_dq,
+ NULL, NULL, 0, "set disk quota limit for group"},
+
{QCMD(Q_GETQUOTA, GRPQUOTA), &test_id, &res_dq, &set_dq.dqb_bsoftlimit,
- &res_dq.dqb_bsoftlimit, "set disk quota limit for group"},
+ &res_dq.dqb_bsoftlimit, sizeof(res_dq.dqb_bsoftlimit),
+ "set disk quota limit for group"},
# if defined(HAVE_QUOTAV2)
- {QCMD(Q_SETINFO, GRPQUOTA), &test_id, &set_qf, &data, &data,
- "set information about quotafile for group"},
+ {QCMD(Q_SETINFO, GRPQUOTA), &test_id, &set_qf,
+ NULL, NULL, 0, "set information about quotafile for group"},
+
{QCMD(Q_GETINFO, GRPQUOTA), &test_id, &res_qf, &set_qf.dqi_bgrace,
- &res_qf.dqi_bgrace, "get information about quotafile for group"},
- {QCMD(Q_GETFMT, GRPQUOTA), &test_id, fmt_buf, &fmt_id, fmt_buf,
- "get quota format for group"},
+ &res_qf.dqi_bgrace, sizeof(res_qf.dqi_bgrace),
+ "get information about quotafile for group"},
+
+ {QCMD(Q_GETFMT, GRPQUOTA), &test_id, &fmt_buf,
+ &fmt_id, &fmt_buf, sizeof(fmt_buf), "get quota format for group"},
# endif
- {QCMD(Q_SYNC, GRPQUOTA), &test_id, &res_dq, &data, &data,
- "update quota usages for group"},
- {QCMD(Q_QUOTAOFF, GRPQUOTA), &test_id, GRPPATH, &data, &data,
- "turn off quota for group"}
+ {QCMD(Q_SYNC, GRPQUOTA), &test_id, &res_dq,
+ NULL, NULL, 0, "update quota usages for group"},
+
+ {QCMD(Q_QUOTAOFF, GRPQUOTA), &test_id, GRPPATH,
+ NULL, NULL, 0, "turn off quota for group"}
};
static void setup(void)
@@ -185,7 +201,7 @@ static void verify_quota(unsigned int n)
res_dq.dqb_bsoftlimit = 0;
res_qf.dqi_igrace = 0;
- memset(fmt_buf, 0, sizeof(fmt_buf));
+ memset(&fmt_buf, 0, sizeof(fmt_buf));
TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr));
if (TEST_RETURN == -1) {
@@ -193,13 +209,14 @@ static void verify_quota(unsigned int n)
return;
}
- if (*tc->set_data != *tc->res_data) {
- tst_res(TFAIL, "quotactl got unexpected info %lu, "
- "expected %lu", *tc->res_data, *tc->set_data);
+ if (memcmp(tc->res_data, tc->set_data, tc->sz)) {
+ tst_res(TFAIL, "quotactl failed to %s", tc->des);
+ tst_res_hexd(TINFO, tc->res_data, tc->sz, "retval: ");
+ tst_res_hexd(TINFO, tc->set_data, tc->sz, "expected: ");
return;
}
- tst_res(TPASS, "quotactl secceeded to %s", tc->des);
+ tst_res(TPASS, "quotactl succeeded to %s", tc->des);
}
static struct tst_test test = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [LTP] [PATCH] quotactl01: replace int comparison with memcmp
2017-01-13 11:43 [LTP] [PATCH] quotactl01: replace int comparison with memcmp Jan Stancek
@ 2017-01-16 2:13 ` Xiao Yang
2017-01-16 11:50 ` Cyril Hrubis
1 sibling, 0 replies; 4+ messages in thread
From: Xiao Yang @ 2017-01-16 2:13 UTC (permalink / raw)
To: ltp
On 2017/01/13 19:43, Jan Stancek wrote:
> Testcase currently uses unsigned long for all returned
> and expected values. This doesn't work for some cases,
> for example Q_GETFMT returns 32bit number. So if we pass
> in unsigned long (8 bytes) on big endian, the comparison
> breaks:
>
> quotactl01.c:198: FAIL: quotactl got unexpected info 8589934592, expected 2
> 2 << 32 == 8589934592
>
> This patch converts parameters to void* and adds size value. After
> syscall it passes all to memcmp.
>
> Patch also removes dummy "data" variable and replaces it with
> NULL and 0 size parameters.
Hi Jan
It looks fine to me, thanks for your patch.
Best Regards,
Xiao Yang
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> testcases/kernel/syscalls/quotactl/quotactl01.c | 99 +++++++++++++++----------
> 1 file changed, 58 insertions(+), 41 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c
> index 586272af1d0c..fb74a8a6404f 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl01.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl01.c
> @@ -79,9 +79,8 @@
> # define MNTPOINT "mntpoint"
>
> static int mount_flag;
> -static unsigned long fmt_id = FMTID;
> -static unsigned long test_id;
> -static unsigned long data;
> +static int32_t fmt_id = FMTID;
> +static int test_id;
> static struct dqblk set_dq = {
> .dqb_bsoftlimit = 100,
> .dqb_valid = QIF_BLIMITS
> @@ -93,53 +92,70 @@ static struct dqinfo set_qf = {
> .dqi_valid = IIF_BGRACE
> };
> static struct dqinfo res_qf;
> -static unsigned long fmt_buf[32];
> +static int32_t fmt_buf;
> # endif
>
> static struct tcase {
> int cmd;
> - unsigned long *id;
> + int *id;
> void *addr;
> - unsigned long *set_data;
> - unsigned long *res_data;
> + void *set_data;
> + void *res_data;
> + int sz;
> char *des;
> } tcases[] = {
> - {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, USRPATH, &data, &data,
> - "turn on quota for user"},
> - {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dq, &data, &data,
> - "set disk quota limit for user"},
> - {QCMD(Q_GETQUOTA, USRQUOTA), &test_id, &res_dq, &set_dq.dqb_bsoftlimit,
> - &res_dq.dqb_bsoftlimit, "get disk quota limit for user"},
> + {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, USRPATH,
> + NULL, NULL, 0, "turn on quota for user"},
> +
> + {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dq,
> + NULL, NULL, 0, "set disk quota limit for user"},
> +
> + {QCMD(Q_GETQUOTA, USRQUOTA), &test_id, &res_dq,
> + &set_dq.dqb_bsoftlimit, &res_dq.dqb_bsoftlimit,
> + sizeof(res_dq.dqb_bsoftlimit), "get disk quota limit for user"},
> # if defined(HAVE_QUOTAV2)
> - {QCMD(Q_SETINFO, USRQUOTA), &test_id, &set_qf, &data, &data,
> - "set information about quotafile for user"},
> - {QCMD(Q_GETINFO, USRQUOTA), &test_id, &res_qf, &set_qf.dqi_bgrace,
> - &res_qf.dqi_bgrace, "get information about quotafile for user"},
> - {QCMD(Q_GETFMT, USRQUOTA), &test_id, fmt_buf, &fmt_id, fmt_buf,
> + {QCMD(Q_SETINFO, USRQUOTA), &test_id, &set_qf,
> + NULL, NULL, 0, "set information about quotafile for user"},
> +
> + {QCMD(Q_GETINFO, USRQUOTA), &test_id, &res_qf,
> + &set_qf.dqi_bgrace, &res_qf.dqi_bgrace, sizeof(res_qf.dqi_bgrace),
> + "get information about quotafile for user"},
> +
> + {QCMD(Q_GETFMT, USRQUOTA), &test_id, &fmt_buf,
> + &fmt_id, &fmt_buf, sizeof(fmt_buf),
> "get quota format for user"},
> # endif
> - {QCMD(Q_SYNC, USRQUOTA), &test_id, &res_dq, &data, &data,
> - "update quota usages for user"},
> - {QCMD(Q_QUOTAOFF, USRQUOTA), &test_id, USRPATH, &data, &data,
> - "turn off quota for user"},
> - {QCMD(Q_QUOTAON, GRPQUOTA), &fmt_id, GRPPATH, &data, &data,
> - "turn on quota for group"},
> - {QCMD(Q_SETQUOTA, GRPQUOTA), &test_id, &set_dq, &data, &data,
> - "set disk quota limit for group"},
> + {QCMD(Q_SYNC, USRQUOTA), &test_id, &res_dq,
> + NULL, NULL, 0, "update quota usages for user"},
> +
> + {QCMD(Q_QUOTAOFF, USRQUOTA), &test_id, USRPATH,
> + NULL, NULL, 0, "turn off quota for user"},
> +
> + {QCMD(Q_QUOTAON, GRPQUOTA), &fmt_id, GRPPATH,
> + NULL, NULL, 0, "turn on quota for group"},
> +
> + {QCMD(Q_SETQUOTA, GRPQUOTA), &test_id, &set_dq,
> + NULL, NULL, 0, "set disk quota limit for group"},
> +
> {QCMD(Q_GETQUOTA, GRPQUOTA), &test_id, &res_dq, &set_dq.dqb_bsoftlimit,
> - &res_dq.dqb_bsoftlimit, "set disk quota limit for group"},
> + &res_dq.dqb_bsoftlimit, sizeof(res_dq.dqb_bsoftlimit),
> + "set disk quota limit for group"},
> # if defined(HAVE_QUOTAV2)
> - {QCMD(Q_SETINFO, GRPQUOTA), &test_id, &set_qf, &data, &data,
> - "set information about quotafile for group"},
> + {QCMD(Q_SETINFO, GRPQUOTA), &test_id, &set_qf,
> + NULL, NULL, 0, "set information about quotafile for group"},
> +
> {QCMD(Q_GETINFO, GRPQUOTA), &test_id, &res_qf, &set_qf.dqi_bgrace,
> - &res_qf.dqi_bgrace, "get information about quotafile for group"},
> - {QCMD(Q_GETFMT, GRPQUOTA), &test_id, fmt_buf, &fmt_id, fmt_buf,
> - "get quota format for group"},
> + &res_qf.dqi_bgrace, sizeof(res_qf.dqi_bgrace),
> + "get information about quotafile for group"},
> +
> + {QCMD(Q_GETFMT, GRPQUOTA), &test_id, &fmt_buf,
> + &fmt_id, &fmt_buf, sizeof(fmt_buf), "get quota format for group"},
> # endif
> - {QCMD(Q_SYNC, GRPQUOTA), &test_id, &res_dq, &data, &data,
> - "update quota usages for group"},
> - {QCMD(Q_QUOTAOFF, GRPQUOTA), &test_id, GRPPATH, &data, &data,
> - "turn off quota for group"}
> + {QCMD(Q_SYNC, GRPQUOTA), &test_id, &res_dq,
> + NULL, NULL, 0, "update quota usages for group"},
> +
> + {QCMD(Q_QUOTAOFF, GRPQUOTA), &test_id, GRPPATH,
> + NULL, NULL, 0, "turn off quota for group"}
> };
>
> static void setup(void)
> @@ -185,7 +201,7 @@ static void verify_quota(unsigned int n)
>
> res_dq.dqb_bsoftlimit = 0;
> res_qf.dqi_igrace = 0;
> - memset(fmt_buf, 0, sizeof(fmt_buf));
> + memset(&fmt_buf, 0, sizeof(fmt_buf));
>
> TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr));
> if (TEST_RETURN == -1) {
> @@ -193,13 +209,14 @@ static void verify_quota(unsigned int n)
> return;
> }
>
> - if (*tc->set_data != *tc->res_data) {
> - tst_res(TFAIL, "quotactl got unexpected info %lu, "
> - "expected %lu", *tc->res_data, *tc->set_data);
> + if (memcmp(tc->res_data, tc->set_data, tc->sz)) {
> + tst_res(TFAIL, "quotactl failed to %s", tc->des);
> + tst_res_hexd(TINFO, tc->res_data, tc->sz, "retval: ");
> + tst_res_hexd(TINFO, tc->set_data, tc->sz, "expected: ");
> return;
> }
>
> - tst_res(TPASS, "quotactl secceeded to %s", tc->des);
> + tst_res(TPASS, "quotactl succeeded to %s", tc->des);
> }
>
> static struct tst_test test = {
^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH] quotactl01: replace int comparison with memcmp
2017-01-13 11:43 [LTP] [PATCH] quotactl01: replace int comparison with memcmp Jan Stancek
2017-01-16 2:13 ` Xiao Yang
@ 2017-01-16 11:50 ` Cyril Hrubis
2017-01-16 13:33 ` Jan Stancek
1 sibling, 1 reply; 4+ messages in thread
From: Cyril Hrubis @ 2017-01-16 11:50 UTC (permalink / raw)
To: ltp
Hi!
> - memset(fmt_buf, 0, sizeof(fmt_buf));
> + memset(&fmt_buf, 0, sizeof(fmt_buf));
Hmm, we can simply do fmt_buf = 0 here now, right?
Otherwise this looks good, acked.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH] quotactl01: replace int comparison with memcmp
2017-01-16 11:50 ` Cyril Hrubis
@ 2017-01-16 13:33 ` Jan Stancek
0 siblings, 0 replies; 4+ messages in thread
From: Jan Stancek @ 2017-01-16 13:33 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it, "yangx jy" <yangx.jy@cn.fujitsu.com>
> Sent: Monday, 16 January, 2017 12:50:57 PM
> Subject: Re: [PATCH] quotactl01: replace int comparison with memcmp
>
> Hi!
> > - memset(fmt_buf, 0, sizeof(fmt_buf));
> > + memset(&fmt_buf, 0, sizeof(fmt_buf));
>
> Hmm, we can simply do fmt_buf = 0 here now, right?
Replaced with simple assignment and pushed.
Regards,
Jan
>
> Otherwise this looks good, acked.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-16 13:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 11:43 [LTP] [PATCH] quotactl01: replace int comparison with memcmp Jan Stancek
2017-01-16 2:13 ` Xiao Yang
2017-01-16 11:50 ` Cyril Hrubis
2017-01-16 13:33 ` Jan Stancek
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.