Re: [PATCH v2] commands: import memtester 4.3.0 from Debian GNU/Linux

[Peter Mamonov <pmamonov@gmail.com>]

Hi, Sascha,

On Wed, Oct 28, 2020 at 10:47:37AM +0100, Sascha Hauer wrote:
> Hi Peter,
> 
> There are a few more things to fix. I just saw I never answered to your
> last mail.

> [...]

> > +    errno = 0;
> > +    wantbytes = (size_t) strtoull_suffix(argv[optind], 0, 0);
> > +    if (errno != 0) {
> > +        printf("failed to parse memory argument");
> > +        return COMMAND_ERROR_USAGE;
> > +    }
> 
> strtoull_suffix() doesn't set errno. There is currently no way to detect
> an error from strtoull_suffix().

errno is checked throughout the original code, including this slightly modified 
piece. Should I remove those checks or keep them for the sake of future errno 
support?

> [...]

> > +        printf("  %-20s: ", "Stuck Address");
> > +        console_flush();
> 
> Why did you add all these console_flush() throughout the code?


I've merely replaced `fflush(stdout)` with `console_flush()` (see the original: 
https://github.com/pmamonov/barebox/blob/31fd9d18f37ed58e5e559fe2ad6686066966f86e/commands/memtester/memtester.c#L374).  
I choosed to keep all those (excessive) flushes as is, since most of the output 
is a progress indicator which should be displayed immediately and I wasn't sure 
that some type of barebox console won't cache the output.


PS Will address other comments in the next patch.

Peter

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