[U-Boot] [PATCH v3 1/2] Loop block device for sandbox

[Pavel Herrmann]

On Thursday 06 of September 2012 03:08:42 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> > On Wednesday 05 of September 2012 14:48:40 Marek Vasut wrote:
> > > Dear Pavel Herrmann,
> > > 
> > > [...]
> > > 
> > > > > besides, I think it'd be much systematic to just scream at user to
> > > > > call "sata rescan" and bail out instead of doing it for him.
> > > > 
> > > > i dont actually need a sata rescan, i just need to make sure i have
> > > > dynamically allocated names
> > > 
> > > Sorry, I can't parse this ... but ...
> > > 
> > > > , so i can safely call free() later, otherwise
> > > > this segfaults when called before sata scan
> > > 
> > > The free() function frees the memory space pointed to by ptr, which must
> > > have been returned by a previous call to malloc(), calloc() or
> > > realloc().
> > > Otherwise, or if free(ptr) has already been called before, undefined
> > > behavior  occurs. If ptr is NULL, no operation is performed.
> > > 
> > > So if you call free() on null pointer, nothing happens. Where's the real
> > > problem?
> > 
> > if you called "sata init" before setting all the loops you would get to
> > open(NULL) and a strlen(NULL). i think its easier to supply valid empty
> > string then check for NULL at every access.
> 
> I'd say check for null on every access ... you'd lower memory requirements
> and the increase in code requirement should really be very minor. Also,
> it'd remove the "zeroed" static variable altogether, correct?

OK, fine by me

> > > > > Make this "info" part mandatory. Than you can cut the whole argc
> > > > > loop
> > > > > into simple "if argc != 2 ; then fail" . And do simple checking for
> > > > > the first letter of the argument being either i or d .
> > > > 
> > > > wont help, still need argc 3 or 4
> > > 
> > > Makes is simpler, you can bail out if it's not 3 or 4
> > 
> > still, i should have a "sata_loop info" work on all files, so theres a
> > valid command with argc 2
> 
> Understood.
> 
> > > > > > +	"sata_loop load devnum file - load file from host FS into loop
> > > > > > devnum"
> > > > > 
> > > > > sata_loop is redundant above.
> > > > 
> > > > really? how do you figure?
> > > 
> > > Run "help sata_loop" and see the result ...
> > 
> > =>help sata_loop
> > sata_loop - SATA loopback
> > 
> > Usage:
> > sata_loop [info] devnum - show info about loop devnum
> > sata_loop load devnum file - load file from host FS into loop devnum
> > 
> > i dont see your problem
> 
> Ewww ... nevermind ... I just detected yet another stupid issue in the
> codebase. I'd say this "sata_loop" should be either omitted and filled by
> the macro or something. But that's for other time to solve ... so I'm fine
> with this now.

it actually is, but only to the first line (as both lines are in one string). 
In my opinion it should either add this for each line (with some magic string 
modification) or none at all, to keep it consistent


Pavel Herrmann