Re: [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance

[Dan Carpenter <dan.carpenter@oracle.com>]

On Tue, Sep 02, 2014 at 04:39:47PM +0530, Sudip Mukherjee wrote:
> fixed sparse warning : context imbalance in 'do_locked_client_insert'
> 			 different lock contexts for basic block
> 
> spin_unlock_irqrestore is called at a later stage before returning 
> from the function if locked is 1.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

This doesn't match the email address you are using.

Really, your patch isn't bad but I would prefer if you re-wrote this
entire function because currently it is garbage.

static u8
do_locked_client_insert(struct uisqueue_info *queueinfo,
                        unsigned int whichqueue,
                        void *pSignal,
                        spinlock_t *lock,
                        unsigned char issueInterruptIfEmpty,
                        u64 interruptHandle, u8 *channelId)
{
        unsigned long flags;
        unsigned char queueWasEmpty;

        spin_lock_irqsave(lock, flags);

        if (!ULTRA_CHANNEL_CLIENT_ACQUIRE_OS(queueinfo->chan, channelId, NULL))
                goto unlock;

        queueWasEmpty = visor_signalqueue_empty(queueinfo->chan, whichqueue);
        if (!visor_signal_insert(queueinfo->chan, whichqueue, pSignal))
                goto release;
        ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL);
        spin_unlock_irqrestore(lock, flags);

        queueinfo->packets_sent++;

        return 1;

release:
        ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL);
unlock:
        spin_unlock_irqrestore(lock, flags);

        return 0;
}

The queueWasEmpty variable is kind of silly.  It should just be an int
or maybe a bool if you are being pedantic but instead we very
specifically set it to be an unsigned variable of the incorrect type.
Also we don't use queueWasEmpty at all.  I think we could delete it...

The problem with the original code was that the error paths and the
success paths were mixed together like spaghetti.  If you separate them
out and unwind in the proper order with normal label names then the
code is easy to understand.

regards,
dan carpenter