lkml.org 
[lkml]   [2011]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] ext4: remove unneeded variable.
    From
    2011/11/20 Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>:
    > On 11/20/2011 01:41 AM, Srivatsa S. Bhat wrote:
    >> On 11/11/2011 08:32 PM, Namjae Jeon wrote:
    >>> ret2 is not needed in ext4_flush_completed_IO().
    >>>
    >>
    >> Not needed? I went through the code briefly, and I don't agree.
    >>
    >>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
    >>> Signed-off-by: Amit Sahrawat <amit.sahrawat83@gmail.com>
    >>> ---
    >>>  fs/ext4/fsync.c |    5 +----
    >>>  1 files changed, 1 insertions(+), 4 deletions(-)
    >>>
    >>> diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
    >>> index 00a2cb7..40397ac 100644
    >>> --- a/fs/ext4/fsync.c
    >>> +++ b/fs/ext4/fsync.c
    >>> @@ -81,7 +81,6 @@ int ext4_flush_completed_IO(struct inode *inode)
    >>>      struct ext4_inode_info *ei = EXT4_I(inode);
    >>>      unsigned long flags;
    >>>      int ret = 0;
    >>> -    int ret2 = 0;
    >>>
    >>>      dump_completed_IO(inode);
    >>>      spin_lock_irqsave(&ei->i_completed_io_lock, flags);
    >>> @@ -105,12 +104,10 @@ int ext4_flush_completed_IO(struct inode *inode)
    >>>               */
    >>>              spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
    >>>              ret = ext4_end_io_nolock(io);
    >>> -            if (ret < 0)
    >>> -                    ret2 = ret;
    >>>              spin_lock_irqsave(&ei->i_completed_io_lock, flags);
    >>>      }
    >>>      spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
    >>> -    return (ret2 < 0) ? ret2 : 0;
    >>> +    return (ret < 0) ? ret : 0;
    >>>  }
    >>
    >> Please note that there is a while loop involved here. Which means, that ret2
    >> is used to store the last negative value of ret. And due to the loop, ret can
    >> be over-written in the next loop iteration, which we can afford, because we
    >> have already stored what we need to save, in ret2. And this ret2 value is used
    >> to return appropriate value to the caller.
    >>
    >
    > Actually, what I really meant was, removing ret2 as merely "unneeded" might not
    > be the right thing to do because once you apply your patch, you end up altering
    > the value returned by this function!
    >
    > If the return value is indeed wrong in the current code, you should rather
    > be saying that this is a bug fix, with appropriate justification IMO.
    >
    Thanks for your review. My thinking was reaching for to while loop().
    I will remember your advice.
    Thanks Srivatsa.
    > Thanks,
    > Srivatsa S. Bhat
    >
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-11-19 22:55    [W:0.030 / U:4.144 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site