All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Chinner <dgc@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: David Chinner <dgc@sgi.com>, xfs@oss.sgi.com, xfs-dev@sgi.com
Subject: Re: [PATCH] Clean up sparse warnings
Date: Wed, 31 Oct 2007 10:03:20 +1100	[thread overview]
Message-ID: <20071030230320.GS66820511@sgi.com> (raw)
In-Reply-To: <20071030100523.GA23489@infradead.org>

On Tue, Oct 30, 2007 at 10:05:23AM +0000, Christoph Hellwig wrote:
> On Tue, Oct 30, 2007 at 10:34:42AM +1100, David Chinner wrote:
> > 
> > Clean up most outstanding sparse warnings.
> > 
> > These are mostly locking annotations, marking things static,
> > casts where needed and declaring stuff in header files.
> 
> Nice.  Note that once we start on making things static there's also a lot
> of things not really used non-static but exported which we should cleanup
> aswell.  I'll look at that when I get some time.
> 
> Note that we'll also always get tons of sparse warnings for debug builds
> because STATIC is defined away..

Yeah, I noticed that - but given that we've done that on purpose to
aid debugging, I don't think it will change ;)

> > @@ -2733,21 +2733,13 @@ xlog_recover_do_efd_trans(
> >  				 * AIL lock.
> >  				 */
> >  				xfs_trans_delete_ail(mp, lip);
> > -				break;
> > +				xfs_efi_item_free(efip);
> > +				return;
> >  			}
> >  		}
> >  		lip = xfs_trans_next_ail(mp, lip, &gen, NULL);
> >  	}
> > -
> > -	/*
> > -	 * If we found it, then free it up.  If it wasn't there, it
> > -	 * must have been overwritten in the log.  Oh well.
> > -	 */
> > -	if (lip != NULL) {
> > -		xfs_efi_item_free(efip);
> > -	} else {
> > -		spin_unlock(&mp->m_ail_lock);
> > -	}
> > +	spin_unlock(&mp->m_ail_lock);
> 
> Imho non-trivial changes like this hunk always deserve beeing a patch of
> it's own where they're described in details.

Ok, I'll split that one out.

> Note that I also get warnings from the lock annotations prover in sparse
> about some conditional locking in xfs_mount.c.  I have patches I still need
> to run through testing for those which clean the code up quite nicely aswell.

Oh, yeah, I left a couple in the icsb code alone as they'd require more than
trivial annotation to fix. I just wanted to remove the majority of the
noise so I could see new problems easily. Patches to fix them would be great
;)

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

      reply	other threads:[~2007-10-30 23:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-29 23:34 [PATCH] Clean up sparse warnings David Chinner
2007-10-30  7:20 ` Lachlan McIlroy
2007-10-30 10:05 ` Christoph Hellwig
2007-10-30 23:03   ` David Chinner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071030230320.GS66820511@sgi.com \
    --to=dgc@sgi.com \
    --cc=hch@infradead.org \
    --cc=xfs-dev@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.