[PATCH 1/8] libblock: Add sparse disk

[Gedare Bloom]

OK. The test case you submitted ought to account for the semaphore though.


On Mon, Dec 3, 2012 at 8:11 AM, Ralf Kirchner <
ralf.kirchner at embedded-brains.de> wrote:

> Hi Gedare,
> Thanks for reviewing my modifications. Most of your comments are fixed
> locally meanwhile. Only these 3 points remain:
>
> Am 29.11.2012 17:55, schrieb Gedare Bloom:
>
> >     +
> >     +  if (NULL == sd)
> >     +    return RTEMS_INVALID_ADDRESS;
> >     +
> >     +  uint8_t *data = (uint8_t *) sd;
> >
> > again why the extra variable?
> In this case the variable data gets incremented in the header of the for
> loop and used within that loop. Thus in this case the variable is a
> simplification.
>
> >     +  sc = rtems_semaphore_create(
> >     +    rtems_build_name('S', 'P', 'A', 'R'),
> >     +    1,
> >     +    RTEMS_PRIORITY | RTEMS_BINARY_SEMAPHORE |
> RTEMS_INHERIT_PRIORITY,
> >     +    0,
> >     +    &sd->mutex
> >     +  );
> >
> > This semaphore should be accounted in application configuration
> > requirements, either explicitly by applications that use the
> > rtems_sparse_disk, or with some macro that detects it..
> I agree with Sebastians comment on this topic. I very much doubt that
> the sparse disk ever will be used anywhere else but within tests.
>
> >     +static rtems_sparse_disk_key *
> >     + sparse_disk_get_new_block(
> >     +  rtems_sparse_disk *sparse_disk,
> >     +  const rtems_blkdev_bnum block
> >     +)
> >     +{
> >     +  rtems_sparse_disk_key *key;
> >     +
> >     +  if (sparse_disk->used_count < sparse_disk->blocks_allocated) {
> >     +    key = &sparse_disk->key_table [sparse_disk->used_count];
> >     +    key->block = block;
> >     +    ++sparse_disk->used_count;
> >     +    qsort(sparse_disk->key_table, sparse_disk->used_count,
> >     sizeof(rtems_sparse_disk_key), sparse_disk_compare);
> >
> > is quicksort every time OK here? is a sorted array the best structure to
> > use, or should a tree be used? Just wondering out loud.
> Using the sparse disk anywhere else but within test does not make much
> sense. And for testing purposes the quicksort approach seems sufficient
> to me.
>
> --
> --------------------------------------------
> Embedded Brains GmbH
> Ralf Kirchner          Dornierstr. 4
> D-82178 Puchheim       Germany
> email: ralf.kirchner at embedded-brains.de
> Phone: +49-89-18 94 741-17
> Fax:   +49-89-18 94 741-08
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20121203/ac74608e/attachment.html>