[Ocfs2-tools-devel] [RFC 1/8] dx_dirs v4: add indexed dirs support in fsck.ocfs2

[Coly Li]

On 2010年01月22日 11:09, Tao Ma Wrote:
> 
> 
> Coly Li wrote:
>> +errcode_t o2fsck_try_add_reidx_dir(struct rb_root *root, uint64_t dino)
>> +{
>> +    errcode_t ret = 1;
>> +    uint64_t ino;
>> +    ino = o2fsck_search_reidx_dir(root, dino);
>> +    if (ino) {
>> +        ret = 0;
>> +        goto out;
>> +    }
>> +    ret = o2fsck_add_reidx_dir_ino(root, dino);
> Joel has pointed that the initializtion of "ret" should be a really
> error. not 1.
> I am just curious why you can't do like this:
>     errcode_t ret = 0;
>     uint64_t ino;
>     ino = o2fsck_search_reidx_dir(root, dino);
>     if (ino)
>         goto out;
> In this case, you don't need to set "ret = 0" if ino is found.
> 
Good idea. Fixed :-)

>> +static errcode_t ocfs2_rebuild_indexed_dir(ocfs2_filesys *fs,
>> uint64_t ino)
[snip]
>> +    if (di->i_dyn_features & OCFS2_INLINE_DATA_FL)
>> +        goto out;
>> +    ocfs2_free(&di_buf);
> this is a bug. you don't reset di_buf here. so in "out" you will double
> free the same memory. Actually you don't need to free di_buf since you
> always exit from "out".

Fixed, thanks for the catching :-)


>> +static errcode_t o2fsck_check_dx_dir(o2fsck_state *ost, struct
>> ocfs2_dinode *di)
>> +{
[snip]
>> +    ret = check_el(ost, &ei, di, &dx_root->dr_list,
>> +            ocfs2_extent_recs_per_dx_root(fs->fs_blocksize),
>> +            &changed);
> Just FYI. check_el changed somehow if my refcount tree get committed.

I see. Once your patch committed, I will update my patch series.

>> +static void release_re_idx_dirs_rbtree(struct rb_root * root)
>> +{
>> +    struct rb_node *node;
>> +    o2fsck_dirblock_entry *dp;
>> +
>> +    while ((node = rb_first(root)) != NULL) {
>> +        dp = rb_entry(node, o2fsck_dirblock_entry, e_node);
>> +        rb_erase(&dp->e_node, root);
>> +        free(dp);
> please use ocfs2_free since you use ocfs2_malloc0.

Fixed.

-- 
Coly Li
SuSE Labs