Linux-NFS Archive mirror
 help / color / mirror / Atom feed
* Re: [LTP] [PATCH] fix rpc_suite/rpc:add check returned value
       [not found] <20210617070806.174220-1-dongshijiang@inspur.com>
@ 2021-06-21  7:59 ` Petr Vorel
  0 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2021-06-21  7:59 UTC (permalink / raw
  To: dongshijiang
  Cc: ltp, Alexey Kodanev, Steve Dickson, libtirpc-devel, linux-nfs

Hi all,

[Cc libtirpc ML and Steve]

> "Segmentation fault (core dumped)" due to the failure of svcfd_create during the rpc test, so you need to check the return value of the "svcfd_create" function

I'm not sure what is the value of TI-RPC tests. IMHO really messy code does
not in the end cover much of libtirpc functionality. That's why I'm thinking
to propose deleting whole testcases/network/rpc/rpc-tirpc/. libtirpc is being
used in nfs-utils, thus it'd deserve to have some testing, but IMHO this
should be libtirpc.

I'm not planning to dive into the technology to understand it enough be
able to written the tests from scratch and I'm not aware of anybody else
planning it.

> Signed-off-by: dongshijiang <dongshijiang@inspur.com>
> ---
>  .../rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c      | 5 +++++
>  .../rpc_createdestroy_svc_destroy/rpc_svc_destroy_stress.c   | 5 +++++
>  .../rpc/rpc_regunreg_xprt_register/rpc_xprt_register.c       | 5 +++++
>  .../rpc/rpc_regunreg_xprt_unregister/rpc_xprt_unregister.c   | 5 +++++
>  4 files changed, 20 insertions(+)

> diff --git a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c
> index 60b96cec3..3557c0068 100644
> --- a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c
> @@ -46,6 +46,11 @@ int main(void)

>  	//First of all, create a server
>  	svcr = svcfd_create(fd, 0, 0);
> +
> +	//check returned value
> +	if ((SVCXPRT *) svcr == NULL) {
> +		return test_status;
> +	}

>  	//Then call destroy macro
>  	svc_destroy(svcr);
> diff --git a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy_stress.c b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy_stress.c
> index ecd145393..5a4331f4d 100644
> --- a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy_stress.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy_stress.c
> @@ -55,6 +55,11 @@ int main(int argn, char *argc[])
>  	//First of all, create a server
>  	for (i = 0; i < nbCall; i++) {
>  		svcr = svcfd_create(fd, 0, 0);
> +
> +		//check returned value
> +		if ((SVCXPRT *) svcr == NULL)
> +			continue;
> +		svcr = NULL;
man svc_destroy(3) states that it deallocates private data structures, including
xprt itself.

Kind regards,
Petr

>  		//Then call destroy macro
>  		svc_destroy(svcr);
> diff --git a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_register/rpc_xprt_register.c b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_register/rpc_xprt_register.c
> index da3b93022..de4df15f1 100644
> --- a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_register/rpc_xprt_register.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_register/rpc_xprt_register.c
> @@ -48,6 +48,11 @@ int main(void)

>  	//create a server
>  	svcr = svcfd_create(fd, 1024, 1024);
> +
> +	//check returned value
> +	if ((SVCXPRT *) svcr == NULL) {
> +		return test_status;
> +	}

>  	//call routine
>  	xprt_register(svcr);
> diff --git a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_unregister/rpc_xprt_unregister.c b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_unregister/rpc_xprt_unregister.c
> index d0b7a20d4..fbaec25ad 100644
> --- a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_unregister/rpc_xprt_unregister.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunreg_xprt_unregister/rpc_xprt_unregister.c
> @@ -52,6 +52,11 @@ int main(int argn, char *argc[])

>  	//create a server
>  	svcr = svcfd_create(fd, 1024, 1024);
> +
> +	//check returned value
> +	if ((SVCXPRT *) svcr == NULL) {
> +		return test_status;
> +	}

>  	xprt_register(svcr);
>  	//call routine

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] fix rpc_suite/rpc:add check returned value
@ 2021-06-21  9:11 James Dong (董世江)
  2021-06-23 16:06 ` Petr Vorel
  0 siblings, 1 reply; 4+ messages in thread
From: James Dong (董世江) @ 2021-06-21  9:11 UTC (permalink / raw
  To: Petr Vorel
  Cc: ltp@lists.linux.it, Alexey Kodanev, Steve Dickson,
	libtirpc-devel@lists.sourceforge.net, linux-nfs@vger.kernel.org

Hi Petr
I think this is just a simple test of some APIs, but some test cases are not standardized and cause errors like "Segmentation fault" during testing. I think it is necessary to fix these errors or delete these tests.

Kind regards,
Dong

-----邮件原件-----
发件人: Petr Vorel [mailto:pvorel@suse.cz] 
发送时间: 2021年6月21日 15:59
收件人: James Dong (董世江) <dongshijiang@inspur.com>
抄送: ltp@lists.linux.it; Alexey Kodanev <aleksei.kodanev@bell-sw.com>; Steve Dickson <SteveD@redhat.com>; libtirpc-devel@lists.sourceforge.net; linux-nfs@vger.kernel.org
主题: Re: [LTP] [PATCH] fix rpc_suite/rpc:add check returned value

Hi all,

[Cc libtirpc ML and Steve]

> "Segmentation fault (core dumped)" due to the failure of svcfd_create 
> during the rpc test, so you need to check the return value of the 
> "svcfd_create" function

I'm not sure what is the value of TI-RPC tests. IMHO really messy code does not in the end cover much of libtirpc functionality. That's why I'm thinking to propose deleting whole testcases/network/rpc/rpc-tirpc/. libtirpc is being used in nfs-utils, thus it'd deserve to have some testing, but IMHO this should be libtirpc.

I'm not planning to dive into the technology to understand it enough be able to written the tests from scratch and I'm not aware of anybody else planning it.

> Signed-off-by: dongshijiang <dongshijiang@inspur.com>
> ---
>  .../rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c      | 5 +++++
>  .../rpc_createdestroy_svc_destroy/rpc_svc_destroy_stress.c   | 5 +++++
>  .../rpc/rpc_regunreg_xprt_register/rpc_xprt_register.c       | 5 +++++
>  .../rpc/rpc_regunreg_xprt_unregister/rpc_xprt_unregister.c   | 5 +++++
>  4 files changed, 20 insertions(+)

> diff --git 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_created
> estroy_svc_destroy/rpc_svc_destroy.c 
> b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_created
> estroy_svc_destroy/rpc_svc_destroy.c
> index 60b96cec3..3557c0068 100644
> --- 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_created
> estroy_svc_destroy/rpc_svc_destroy.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_cre
> +++ atedestroy_svc_destroy/rpc_svc_destroy.c
> @@ -46,6 +46,11 @@ int main(void)

>  	//First of all, create a server
>  	svcr = svcfd_create(fd, 0, 0);
> +
> +	//check returned value
> +	if ((SVCXPRT *) svcr == NULL) {
> +		return test_status;
> +	}

>  	//Then call destroy macro
>  	svc_destroy(svcr);
> diff --git 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_created
> estroy_svc_destroy/rpc_svc_destroy_stress.c 
> b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_created
> estroy_svc_destroy/rpc_svc_destroy_stress.c
> index ecd145393..5a4331f4d 100644
> --- 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_created
> estroy_svc_destroy/rpc_svc_destroy_stress.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_cre
> +++ atedestroy_svc_destroy/rpc_svc_destroy_stress.c
> @@ -55,6 +55,11 @@ int main(int argn, char *argc[])
>  	//First of all, create a server
>  	for (i = 0; i < nbCall; i++) {
>  		svcr = svcfd_create(fd, 0, 0);
> +
> +		//check returned value
> +		if ((SVCXPRT *) svcr == NULL)
> +			continue;
> +		svcr = NULL;
man svc_destroy(3) states that it deallocates private data structures, including xprt itself.

Kind regards,
Petr

>  		//Then call destroy macro
>  		svc_destroy(svcr);
> diff --git 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunre
> g_xprt_register/rpc_xprt_register.c 
> b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunre
> g_xprt_register/rpc_xprt_register.c
> index da3b93022..de4df15f1 100644
> --- 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunre
> g_xprt_register/rpc_xprt_register.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_reg
> +++ unreg_xprt_register/rpc_xprt_register.c
> @@ -48,6 +48,11 @@ int main(void)

>  	//create a server
>  	svcr = svcfd_create(fd, 1024, 1024);
> +
> +	//check returned value
> +	if ((SVCXPRT *) svcr == NULL) {
> +		return test_status;
> +	}

>  	//call routine
>  	xprt_register(svcr);
> diff --git 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunre
> g_xprt_unregister/rpc_xprt_unregister.c 
> b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunre
> g_xprt_unregister/rpc_xprt_unregister.c
> index d0b7a20d4..fbaec25ad 100644
> --- 
> a/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_regunre
> g_xprt_unregister/rpc_xprt_unregister.c
> +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_reg
> +++ unreg_xprt_unregister/rpc_xprt_unregister.c
> @@ -52,6 +52,11 @@ int main(int argn, char *argc[])

>  	//create a server
>  	svcr = svcfd_create(fd, 1024, 1024);
> +
> +	//check returned value
> +	if ((SVCXPRT *) svcr == NULL) {
> +		return test_status;
> +	}

>  	xprt_register(svcr);
>  	//call routine

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] fix rpc_suite/rpc:add check returned value
  2021-06-21  9:11 James Dong (董世江)
@ 2021-06-23 16:06 ` Petr Vorel
  0 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2021-06-23 16:06 UTC (permalink / raw
  To: James Dong (董世江)
  Cc: ltp@lists.linux.it, Alexey Kodanev, Steve Dickson,
	libtirpc-devel@lists.sourceforge.net, linux-nfs@vger.kernel.org

Hi Dong,

> Hi Petr
> I think this is just a simple test of some APIs, but some test cases are not standardized and cause errors like "Segmentation fault" during testing. I think it is necessary to fix these errors or delete these tests.

Sure this fix can get in. I saw issues with some tests on openSUSE, but that's a
separate problem (I was not able to find the problem thus report it.

> Kind regards,
> Dong

> > +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_createdestroy_svc_destroy/rpc_svc_destroy.c
> > @@ -46,6 +46,11 @@ int main(void)

> >  	//First of all, create a server
> >  	svcr = svcfd_create(fd, 0, 0);
> > +
> > +	//check returned value
> > +	if ((SVCXPRT *) svcr == NULL) {
IMHO casting is not required, right? Just
	if (svcr == NULL) {

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] fix rpc_suite/rpc:add check returned value
@ 2021-06-24  1:02 James Dong (董世江)
  0 siblings, 0 replies; 4+ messages in thread
From: James Dong (董世江) @ 2021-06-24  1:02 UTC (permalink / raw
  To: Petr Vorel
  Cc: ltp@lists.linux.it, Alexey Kodanev, Steve Dickson,
	libtirpc-devel@lists.sourceforge.net, linux-nfs@vger.kernel.org

Hi Petr

My test environment is centos8.2, kernel version is 4.18 (CentOS-8.2.2004-x86_64-dvd1.iso)
For example:
	svcr = svcfd_create(fd, 0, 0);
	//Then call destroy macro
	svc_destroy(svcr);

If "svcfd_create" fails, that is, the return value is NULL and then call "svc_destroy" will report an error "Segmentation fault"

Kind regards,
Dong

-----邮件原件-----
发件人: Petr Vorel [mailto:pvorel@suse.cz] 
发送时间: 2021年6月24日 0:07
收件人: James Dong (董世江) <dongshijiang@inspur.com>
抄送: ltp@lists.linux.it; Alexey Kodanev <aleksei.kodanev@bell-sw.com>; Steve Dickson <SteveD@redhat.com>; libtirpc-devel@lists.sourceforge.net; linux-nfs@vger.kernel.org
主题: Re: [LTP] [PATCH] fix rpc_suite/rpc:add check returned value

Hi Dong,

> Hi Petr
> I think this is just a simple test of some APIs, but some test cases are not standardized and cause errors like "Segmentation fault" during testing. I think it is necessary to fix these errors or delete these tests.

Sure this fix can get in. I saw issues with some tests on openSUSE, but that's a separate problem (I was not able to find the problem thus report it.

> Kind regards,
> Dong

> > +++ b/testcases/network/rpc/rpc-tirpc/tests_pack/rpc_suite/rpc/rpc_c
> > +++ reatedestroy_svc_destroy/rpc_svc_destroy.c
> > @@ -46,6 +46,11 @@ int main(void)

> >  	//First of all, create a server
> >  	svcr = svcfd_create(fd, 0, 0);
> > +
> > +	//check returned value
> > +	if ((SVCXPRT *) svcr == NULL) {
IMHO casting is not required, right? Just
	if (svcr == NULL) {

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-24  1:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20210617070806.174220-1-dongshijiang@inspur.com>
2021-06-21  7:59 ` [LTP] [PATCH] fix rpc_suite/rpc:add check returned value Petr Vorel
2021-06-21  9:11 James Dong (董世江)
2021-06-23 16:06 ` Petr Vorel
  -- strict thread matches above, loose matches on Subject: below --
2021-06-24  1:02 James Dong (董世江)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).