Re: [PATCH 1/2] btrfs: fix sparse address space warnings

[David Sterba]

On Mon, Sep 29, 2014 at 12:45:12PM -0700, Omar Sandoval wrote:
> > > @@ -639,8 +640,7 @@ static int send_header(struct send_ctx *sctx)
> > > -	return write_buf(sctx->send_filp, &hdr, sizeof(hdr),
> > > -					&sctx->send_off);
> > > +	return write_buf(sctx->send_filp, &hdr, sizeof(hdr), &sctx->send_off);
> > 
> > >  	ret = write_buf(sctx->send_filp, sctx->send_buf, sctx->send_size,
> > > -					&sctx->send_off);
> > > +			&sctx->send_off);
> > 
> > Please do not fold unrelated changes.
> 
> My metric for "related" here was that these were call sites of a function I
> directly modified.

The changes are only in the whitespace, that's not necessary. It's
usually ok to fix style issues in the code you modify directly.

> Is the preferred form to just split style fixes that we encounter into
> a separate patch in the series?

Well, I may only express my point of view. Yes, split the style-only
changes into another patch and don't send it :)

The problem with patches that do not effectively change anything is that
they pollute git history and just add extra step when one has to look
for a patch that broke something, or eg. change context of following
patches and make backporting a bit more tedious. Code cleanups are fine,
but there's usually a point of making the code more readable, compact,
etc.

The coding style should be perfect from the beginning. Nobody will
probably point out minor style violations during review, because it just
pointless for a patch that fixes a real bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/