diff mbox series

[v10,1/2] cmdline: handle EOF in cmdline_poll

Message ID 20230130200914.22049-2-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series testpmd: handle signals safely |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Jan. 30, 2023, 8:09 p.m. UTC 
  If end of file is reached on input, then cmdline_read_char()
will return 0. The problem is that cmdline_poll() was not checking
for this and would continue and not return the status.

Fixes: 9251cd97a6be ("cmdline: add internal wrappers for character input")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/cmdline/cmdline.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ferruh Yigit Jan. 30, 2023, 10:12 p.m. UTC | #1 
On 1/30/2023 8:09 PM, Stephen Hemminger wrote:
> If end of file is reached on input, then cmdline_read_char()
> will return 0. The problem is that cmdline_poll() was not checking
> for this and would continue and not return the status.
> 
> Fixes: 9251cd97a6be ("cmdline: add internal wrappers for character input")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/cmdline/cmdline.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
> index e1009ba4c413..de41406d61e0 100644
> --- a/lib/cmdline/cmdline.c
> +++ b/lib/cmdline/cmdline.c
> @@ -194,7 +194,7 @@ cmdline_poll(struct cmdline *cl)
>  	else if (status > 0) {
>  		c = -1;
>  		read_status = cmdline_read_char(cl, &c);
> -		if (read_status < 0)
> +		if (read_status <= 0)
>  			return read_status;

According API doc it will be wrong to return '0', which imply 'RDLINE_INIT'.

But function may return any negative value on error, what about to get
eof as and error case:

if (read_status < 0)
	return read_status;
else if (read_status == 0)
	return -EIO;

With this 'cmdline_poll()' can be used in testpmd as it is used in v9 of
this patch:
while (f_quit == 0 && cl_quit == 0) {
	if (cmdline_poll(cl) < 0)
		break;
}



But still I guess this is an ABI break because of API behavior change.
Stephen Hemminger Jan. 31, 2023, 2:54 a.m. UTC | #2 
On Mon, 30 Jan 2023 22:12:42 +0000
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 1/30/2023 8:09 PM, Stephen Hemminger wrote:
> > If end of file is reached on input, then cmdline_read_char()
> > will return 0. The problem is that cmdline_poll() was not checking
> > for this and would continue and not return the status.
> > 
> > Fixes: 9251cd97a6be ("cmdline: add internal wrappers for character input")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/cmdline/cmdline.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
> > index e1009ba4c413..de41406d61e0 100644
> > --- a/lib/cmdline/cmdline.c
> > +++ b/lib/cmdline/cmdline.c
> > @@ -194,7 +194,7 @@ cmdline_poll(struct cmdline *cl)
> >  	else if (status > 0) {
> >  		c = -1;
> >  		read_status = cmdline_read_char(cl, &c);
> > -		if (read_status < 0)
> > +		if (read_status <= 0)
> >  			return read_status;  
> 
> According API doc it will be wrong to return '0', which imply 'RDLINE_INIT'.

The API doc is a mess. It says function returns things enum that is only
defined in cmdline_private.h. Therefore no application could safely depend
on it.

End of File is not an error in most API's.
diff mbox series

Patch

diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index e1009ba4c413..de41406d61e0 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -194,7 +194,7 @@  cmdline_poll(struct cmdline *cl)
 	else if (status > 0) {
 		c = -1;
 		read_status = cmdline_read_char(cl, &c);
-		if (read_status < 0)
+		if (read_status <= 0)
 			return read_status;
 
 		status = cmdline_in(cl, &c, 1);