quotactl01: replace int comparison with memcmp
PATCH 1/1 quotactl01: replace int comparison with memcmp
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.
--- testcases/kernel/syscalls/quotactl/quotactl01.c | 99 +++++++++++++++---------- 1 file changed, 58 insertions(+), 41 deletions(-)
testcases/kernel/syscalls/quotactl/quotactl01.c +58 -41
@@ -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 = {
Signature · 1 line
1.8.3.1
PATCH 1/1 quotactl01: replace int comparison with memcmp
Quoted 6 lines · Jan Stancek
Testcase currently uses unsigned long for all returned and expected values. This doesn't work for some cases, for example
Q_GETFMTreturns 32bit number. So if we pass in unsigned long (8 bytes) on big endian, the comparison breaks:
Quoted 8 lines
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
Quoted 153 lines
Signed-off-by Jan Stancek <[email protected]>--- testcases/kernel/syscalls/quotactl/quotactl01.c | 99 +++++++++++++++---------- 1 file changed, 58 insertions(+), 41 deletions(-)testcases/kernel/syscalls/quotactl/quotactl01.c +58
-41@@ -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 = {
PATCH 1/1 quotactl01: replace int comparison with memcmp
In reply to Jan StancekHi!
testcases/kernel/syscalls/quotactl/quotactl01.c static void verify_quota(unsigned int n) · line 204
Quoted 2 lines
testcases/kernel/syscalls/quotactl/quotactl01.c +1
-1-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.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 quotactl01: replace int comparison with memcmp
----- Original Message -----
Quoted 11 lines
From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected], "yangx jy" <[email protected]>
Sent: Monday, 16 January, 2017 12:50:57 PM
Subject: Re: [PATCH] quotactl01: replace int comparison with memcmpHi!
testcases/kernel/syscalls/quotactl/quotactl01.c static void verify_quota(unsigned int n) · line 204
Quoted 2 lines
testcases/kernel/syscalls/quotactl/quotactl01.c +1
-1-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
Quoted 7 lines
Otherwise this looks good, acked.
--
Cyril Hrubis
[email protected]