Re: [PATCH] orangefs: remove redundant assignment to variable buffer_index

Re: [PATCH] orangefs: remove redundant assignment to variable buffer_index

[Mike Marshall]

>> The only explanation I can think of is that you guys are discussing
>> different code. :P

My response contained several conflations :-) ...

The code in file.c that Colin has flagged does indeed have buffer_index
being initialized needlessly, and the assignment noted by Dan is also
needless. There's even a second needless assignment done in another
place in the same function. While the code around them has changed over
time, these now needless manipulations of buffer_index are not new. I'll
get rid of them.

>> You often send these patches before they hit linux-next so I had skipped
>> reviewing this one when you sent it.

I know Linus is likely to refuse pull requests for stuff that
has not been through linux-next, so I make sure stuff has been
there at least a few days before asking for it to be pulled.
"A few days" is long enough for robots to see it, perhaps not
long enough for humans. I especially appreciate the human review. One of
the good things about Orangefs is that it is easy to install and configure,
especially for testing. Documentation/filesystems/orangefs.txt has
instructions for dnf installing orangefs on Fedora, and also how to download
a source tarball and install from that.

-Mike

On Tue, May 21, 2019 at 11:04 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Thu, May 16, 2019 at 12:06:31PM -0400, Mike Marshall wrote:
> > Hi Colin...
> >
> > Thanks for the patch. Before I initialized buffer_index, Dan Williams sent
> > in a warning that a particular error path could try to use ibuffer_index
> > uninitialized. I could induce the problem he described with one
> > of the xfstests resulting in a crashed kernel. I will try to refactor
> > the code to fix the problem some other way than initializing
> > buffer_index in the declaration.
> >
>
> The only explanation I can think of is that you guys are discussing
> different code.  :P
>
> regards,
> dan carpenter
>

Re: [PATCH] orangefs: remove redundant assignment to variable buffer_index

[Dan Carpenter]

On Tue, Jun 25, 2019 at 02:55:11PM -0400, Mike Marshall wrote:
> >> You often send these patches before they hit linux-next so I had skipped
> >> reviewing this one when you sent it.
> 
> I know Linus is likely to refuse pull requests for stuff that
> has not been through linux-next, so I make sure stuff has been
> there at least a few days before asking for it to be pulled.
> "A few days" is long enough for robots to see it, perhaps not
> long enough for humans. I especially appreciate the human review. One of
> the good things about Orangefs is that it is easy to install and configure,
> especially for testing. Documentation/filesystems/orangefs.txt has
> instructions for dnf installing orangefs on Fedora, and also how to download
> a source tarball and install from that.

No, no, that comment was to Colin.  It's good that he's sending patches
for all the trees as soon as possible like the zero day bot does.  But
it does make it hard to review at times.

regards,
dan carpenter

Re: [PATCH] orangefs: remove redundant assignment to variable buffer_index

[Colin Ian King]

On 26/06/2019 07:18, Dan Carpenter wrote:
> On Tue, Jun 25, 2019 at 02:55:11PM -0400, Mike Marshall wrote:
>>>> You often send these patches before they hit linux-next so I had skipped
>>>> reviewing this one when you sent it.
>>
>> I know Linus is likely to refuse pull requests for stuff that
>> has not been through linux-next, so I make sure stuff has been
>> there at least a few days before asking for it to be pulled.
>> "A few days" is long enough for robots to see it, perhaps not
>> long enough for humans. I especially appreciate the human review. One of
>> the good things about Orangefs is that it is easy to install and configure,
>> especially for testing. Documentation/filesystems/orangefs.txt has
>> instructions for dnf installing orangefs on Fedora, and also how to download
>> a source tarball and install from that.
> 
> No, no, that comment was to Colin.  It's good that he's sending patches
> for all the trees as soon as possible like the zero day bot does.  But
> it does make it hard to review at times.
> 
> regards,
> dan carpenter
> 
Indeed, I normally work against the latest code landing in linux-next,
so apologies for any confusion.  Anyhow, by the look of it these minor
nitpicks still need addressing (really low priority), so shall I leave
that with you Mike to sort out?

Colin