[PATCH 1/8] libblock: Add sparse disk

Ralf Kirchner ralf.kirchner at embedded-brains.de
Mon Dec 3 13:11:03 UTC 2012


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.



More information about the devel mailing list