SoC RAID VFS Project

[Ming Wong]

Hi Jerry,

On 06/07/06, Gerald (Jerry) Carter <jerry at samba.org> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ming,
>
> > BTW, I've implemented a bit of the mkdir() function. It's
> > now able to allow directory mirroring, meaning it creates a
> > copy of a newly created directory in the secondary disk
> > as well.
> >
> > I'll report to samba-technical list in the next few days
> > or next monday when I get a bit more stuff done.
>
> Ming,
>
> Glad to see things.  Couple of comments.
>
> The snippet should be rewritten
>
>    newDisksStr = talloc_size( NULL, strlen(disksStr) + 1);
>    memcpy(newDisksStr, disksStr, strlen(disksStr) + 1);
>
> to
>
>    if ( (newDisksStr = talloc_strdup( NULL, disksStr)) == NULL ) {
>        return error;
>    }
>
> Make sure to check return codes.
>
> Next, the function raid_disks_array_free() is really unnecessary.
> If you handle talloc correctly, you can free the array and all
> pointers with a single TALLOC_FREE().  Remember that any
> talloc()'d memory can be reused as the talloc context for
> allocating more memory.  When you free the main context, all
> subsequent memory is also freed.
>
> This section of code in raid_disks()
>
>    while( (tokens = strtok(NULL, DISKS_DELIMITER)) )
>    {
>       disksArray = (char**)talloc_realloc(NULL, disksArray,
>                                           char**, (counter + 1) );
>       disksArray[counter] = talloc_size(NULL, strlen(tokens) + 1 );
>       memcpy(disksArray[counter], tokens, strlen(tokens) + 1);
>
>       ++counter;
>    }
>
> Should be rewritten
>
>    while( (tokens = strtok(NULL, DISKS_DELIMITER)) )
>    {
>       /* make sure to check return codes here */
>       disksArray = TALLOC_REALLOC_ARRAY(disksArray,
>           disksArray, char**, (counter + 1) );
>       disksArray[counter] = talloc_strdup(disksArray, tokens);
>
>       ++counter;
>    }
>
> Then raid_disks_array_free(disksArray) can be replaced with
> TALLOC_FREE(disksArray).
>
>

Thanks for the comments.
Looks like I really need to correct the way I use talloc()'s. :)
I'll get them fixed soon.

cheers,
Ming