[U-Boot] [PATCH] Loop block device for sandbox

[Pavel Herrmann <morpheus.ibis@gmail.com>]

On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote:
...snip...
> > +extern block_dev_desc_t sata_dev_desc[];
> > +
> > +int init_sata(int dev)
> > +{
> > +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);
> 
> Superfluous braces ... Actually, I think sata_dev_desc as it would work very
> well too.

Straight copy from dwc_ahsata.c, makes it more readable thought, as the order 
of operation is not very intuitive IMHO.

> > +lbaint_t sata_read(int dev, lbaint_t start, lbaint_t blkcnt, void
> > *buffer)
> > +{
> > +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);
> > +	int fd = (long) pdev->priv;
> 
> If pdev is NULL, this will crash

well, it isn't, at least not from the command - thats why you define the number 
of ports in advance, you get "dev" already range-checked

> > +	lbaint_t retval;
> > +
> > +	os_lseek(fd, start*ATA_SECT_SIZE, OS_SEEK_SET);
> > +	retval = os_read(fd, buffer, ATA_SECT_SIZE * blkcnt);
> > +
> > +	return retval/ATA_SECT_SIZE;
> > +}
> > +
> > +lbaint_t sata_write(int dev, lbaint_t start, lbaint_t blkcnt, void
> > *buffer) +{
> > +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);
> > +	int fd = (long) pdev->priv;
> > +	lbaint_t retval;
> > +
> > +	os_lseek(fd, start*ATA_SECT_SIZE, OS_SEEK_SET);
> 
> Besides, lseek can fail, can it not?

If you open a pipe (or nothing), yes
in the first case, you shouldn't, in the second, the I/O op will harmlessly 
fail as well

> > +	if (namelen > 20)
> > +		namelen = 20;
> 
> Why do you trim down the string, won't simple strdup() work?

nah, the destination is char[21], as it is the exact length of corresponding 
field in ATA identify response (one more for a 0 at the end)

> > +	memcpy(pdev->product, filenames[dev], namelen);
> > +	pdev->product[20] = 0;
> > +
> > +	if (fd != -1) {
> 
> And if "fd" is -1 ?

then all defaults to an invalid device, because you failed to open the file, 
for whatever the reason.



"agreed" to the other comments

Best Regards
Pavel Herrmann