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

[Marek Vasut]

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?

> > > > 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.

> Pavel Herrmann

Best regards,
Marek Vasut