lkml.org 
[lkml]   [2012]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] Cleaning up code formatting errors in net/wireless pointed out by checkpatch.
    From
    Date
    On Fri, 2012-02-17 at 11:06 -0800, Stephen Hemminger wrote:

    > > I'd try to make the statement expression visually
    > > distinct. Something like:
    > >
    > > wait_event(rdev->dev_wait,
    > > ({
    > > int __count;
    > > mutex_lock(&rdev->devlist_mtx);
    > > __count = rdev->opencount;
    > > mutex_unlock(&rdev->devlist_mtx);
    > > __count == 0;
    > > })
    > > );
    > >
    >
    > I prefer to see this done as an inline function
    >
    > wait_event(rdev->dev_wait, is_foo_ready(rdev))
    >
    > Also, in this case wrapping a condition with a mutex really is
    > meaningless because the state is longer protected out side the
    > protected region; in other words the mutex here is bogus and
    > provides no additional protection.

    I don't really care about all the changes suggested here -- feel free to
    make them. One thing I'd like to point out though is that generally the
    mutex might serve a purpose even here. In this specific case, it
    currently doesn't, but I still think it's safer to keep it in case
    somebody modifies other code. The case where it matters is when the
    modification of the "opencount" variable isn't the last thing that
    happens in a locked section, but you here want or need to wait for
    everything happening in that section to be done.

    johannes



    \
     
     \ /
      Last update: 2012-02-20 11:49    [W:0.019 / U:36.056 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site