Re: [PATCH 1/2] readline_simple: return -1 if getc fails

["Gaël PORTAY" <gael.portay@savoirfairelinux.com>]

On Tue, Aug 08, 2017 at 05:05:35PM +0100, Ian Abbott wrote:
> On 08/08/17 16:36, Lucas Stach wrote:
> > Am Dienstag, den 08.08.2017, 11:20 -0400 schrieb Gaël PORTAY:
> > > Hi Lucas,
> > > 
> > > On Tue, Aug 08, 2017 at 09:51:54AM +0200, Lucas Stach wrote:
> > > > (...)
> > > > I don't like made up error codes. Is there any reason why we couldn't
> > > > just pass through the negative error code from getchar?
> > > > 
> > > 
> > > The thing here is that getchar() may return an error, and that error is not
> > > tested. This causes readline to print the character 0xea (-EINVAL) which is not
> > > printable.
> > 
> > So why wouldn't the following fix the issue?
> > 
> > signed char c;
> 
> `int` would be better to allow non-ASCII characters.
> 
> > 
> > if (c < 0)
> > 	return c;
> 
> There are places where the return value is checked for `-1` for example in
> get_user_input() ("common/hush.c"), and in run_shell() ("common/parser.c").
> 
> I think Gaël's patch is reasonable, although perhaps it should also set
> `line[0] = '\0';` before returning.
> 

Indeed, or `line[n] = '\0';` to preserve characters already entered.

BTW, it already performed by get_user_input in hush.c (n is reset to 0).

console_buffer[n] = '\n';
console_buffer[n + 1]= '\0';

> Off topic: there is another oddity in the the "simple" version of
> readline(). It ignores the `len` parameter and uses `CONFIG_CBSIZE` instead.
> 

I made a patch for this; but I have not tested yet all cases.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox