lkml.org 
[lkml]   [2008]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [3/3] POHMELFS high performance network filesystem.
    Hi,

    I have just one question yet :-)

    On Fri, Jun 13, 2008 at 6:42 PM, Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
    > +int pohmelfs_copy_config(struct pohmelfs_sb *psb)
    > +{
    > + struct pohmelfs_config *c, *dst;
    > + int err = -ENODEV;
    > +
    > + mutex_lock(&pohmelfs_config_lock);
    > + list_for_each_entry(c, &pohmelfs_config_list, config_entry) {
    > + if (c->state.ctl.idx != psb->idx)
    > + continue;
    > +
    > + err = 0;
    > + list_for_each_entry(dst, &psb->state_list, config_entry) {
    > + if (pohmelfs_config_eql(&dst->state.ctl, &c->state.ctl)) {
    > + err = -EEXIST;
    > + break;
    > + }
    > + }
    > +
    > + if (err)
    > + continue;
    > +
    > + dst = kzalloc(sizeof(struct pohmelfs_config), GFP_KERNEL);
    > + if (!dst) {
    > + err = -ENOMEM;
    > + goto err_out_unlock;
    > + }
    > +
    > + memcpy(&dst->state.ctl, &c->state.ctl, sizeof(struct pohmelfs_ctl));
    > +
    > + list_add_tail(&dst->config_entry, &psb->state_list);
    > +
    > + err = pohmelfs_state_init_one(psb, dst);
    > + if (err) {
    > + list_del(&dst->config_entry);
    > + kfree(dst);
    > + }
    > + }
    > + mutex_unlock(&pohmelfs_config_lock);
    > +
    > + return err;
    > +
    > +err_out_unlock:
    > + mutex_unlock(&pohmelfs_config_lock);
    > +
    > + mutex_lock(&psb->state_lock);
    > + list_for_each_entry_safe(dst, c, &psb->state_list, config_entry) {
    > + list_del(&dst->config_entry);
    > + kfree(dst);
    > + }
    > + mutex_unlock(&psb->state_lock);
    > +
    > + return err;
    > +}

    I'm having a hard time convincing myself that the error handling here
    is correct. You have this kind of setup:

    1. for each config in config list {
    2. for each config in superblock state list {
    pohmelfs_config_eql();
    ...
    }
    }

    And according to your code, if pohmelfs_config_eql returns non-zero in
    the last iteration of #1, then -EEXISTS will be the return value of
    the whole function (but the config _will_ be copied; it is not undone
    in this case). But if pohmenlfs_config_eql returns non-zero in any but
    the last iteration of #1, then 0 will be the return value. Is this
    your intention?


    Vegard

    --
    "The animistic metaphor of the bug that maliciously sneaked in while
    the programmer was not looking is intellectually dishonest as it
    disguises that the error is the programmer's own creation."
    -- E. W. Dijkstra, EWD1036


    \
     
     \ /
      Last update: 2008-06-15 09:51    [W:4.915 / U:0.944 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site