RAID0 - unitialised zone->dev_start for zone[0]

RAID0 - unitialised zone->dev_start for zone[0]

[SandeepKsinha]

Hi Neil,


[22:33:46 sinhas]$ diff raid0.c raid0.c.orig
160d159
<       zone->dev_start = 0;
[/usr/src/linux-2.6.29/drivers/md]
[22:33:59 sinhas]$

I see this piece of code missing and being used later.
Not very sure of the code flow, but still as part of defensive programming.

Am i missing something?



-- 
Regards,
Sandeep.





 	
“To learn is to change. Education is a process that changes the learner.”
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RAID0 - unitialised zone->dev_start for zone[0]

[NeilBrown]

On Thu, May 14, 2009 3:08 am, SandeepKsinha wrote:
> Hi Neil,
>
>
> [22:33:46 sinhas]$ diff raid0.c raid0.c.orig

Always, always, always use "diff -u".  It provides context
so the diff is much easier to read.

> 160d159
> <       zone->dev_start = 0;
> [/usr/src/linux-2.6.29/drivers/md]
> [22:33:59 sinhas]$
>
> I see this piece of code missing and being used later.
> Not very sure of the code flow, but still as part of defensive
> programming.
>
> Am i missing something?

conf->strip_zone is allocated with kzalloc, so all values are
initialised to 0.

NeilBrown


>
>
>
> --
> Regards,
> Sandeep.
>
>
>
>
>
>
> “To learn is to change. Education is a process that changes the learner.”
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RAID0 - unitialised zone->dev_start for zone[0]

[SandeepKsinha]

On Thu, May 14, 2009 at 2:41 AM, NeilBrown <neilb@suse.de> wrote:
> On Thu, May 14, 2009 3:08 am, SandeepKsinha wrote:
>> Hi Neil,
>>
>>
>> [22:33:46 sinhas]$ diff raid0.c raid0.c.orig
>
> Always, always, always use "diff -u".  It provides context
> so the diff is much easier to read.
>
>> 160d159
>> <       zone->dev_start = 0;
>> [/usr/src/linux-2.6.29/drivers/md]
>> [22:33:59 sinhas]$
>>
>> I see this piece of code missing and being used later.
>> Not very sure of the code flow, but still as part of defensive
>> programming.
>>
>> Am i missing something?
>
> conf->strip_zone is allocated with kzalloc, so all values are
> initialised to 0.
>

zone->zone_start = 0;

Sorry, this confused me.

> NeilBrown
>
>
>>
>>
>>
>> --
>> Regards,
>> Sandeep.
>>
>>
>>
>>
>>
>>
>> “To learn is to change. Education is a process that changes the learner.”
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>



-- 
Regards,
Sandeep.





 	
“To learn is to change. Education is a process that changes the learner.”
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RAID0 - unitialised zone->dev_start for zone[0]

[NeilBrown]

On Thu, May 14, 2009 1:14 pm, SandeepKsinha wrote:
> On Thu, May 14, 2009 at 2:41 AM, NeilBrown <neilb@suse.de> wrote:
>> On Thu, May 14, 2009 3:08 am, SandeepKsinha wrote:
>>> Hi Neil,
>>>
>>>
>>> [22:33:46 sinhas]$ diff raid0.c raid0.c.orig
>>
>> Always, always, always use "diff -u".  It provides context
>> so the diff is much easier to read.
>>
>>> 160d159
>>> <       zone->dev_start = 0;
>>> [/usr/src/linux-2.6.29/drivers/md]
>>> [22:33:59 sinhas]$
>>>
>>> I see this piece of code missing and being used later.
>>> Not very sure of the code flow, but still as part of defensive
>>> programming.
>>>
>>> Am i missing something?
>>
>> conf->strip_zone is allocated with kzalloc, so all values are
>> initialised to 0.
>>
>
> zone->zone_start = 0;
>
> Sorry, this confused me.

Are you saying that you think this code is unnecessary?
Yes, you are right.

It is indeed inconsistent to set zone_start to zero but not to
set dev_start to zero.  It is the sort of thing that is probably
worth fixing (I'd be inclined to set them both, it makes the code
clearer) if you are making other changes to code in that area,
but probably isn't worth fixing just by itself.

NeilBrown


--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RAID0 - unitialised zone->dev_start for zone[0]

[SandeepKsinha]

On Thu, May 14, 2009 at 9:54 AM, NeilBrown <neilb@suse.de> wrote:
> On Thu, May 14, 2009 1:14 pm, SandeepKsinha wrote:
>> On Thu, May 14, 2009 at 2:41 AM, NeilBrown <neilb@suse.de> wrote:
>>> On Thu, May 14, 2009 3:08 am, SandeepKsinha wrote:
>>>> Hi Neil,
>>>>
>>>>
>>>> [22:33:46 sinhas]$ diff raid0.c raid0.c.orig
>>>
>>> Always, always, always use "diff -u".  It provides context
>>> so the diff is much easier to read.
>>>
>>>> 160d159
>>>> <       zone->dev_start = 0;
>>>> [/usr/src/linux-2.6.29/drivers/md]
>>>> [22:33:59 sinhas]$
>>>>
>>>> I see this piece of code missing and being used later.
>>>> Not very sure of the code flow, but still as part of defensive
>>>> programming.
>>>>
>>>> Am i missing something?
>>>
>>> conf->strip_zone is allocated with kzalloc, so all values are
>>> initialised to 0.
>>>
>>
>> zone->zone_start = 0;
>>
>> Sorry, this confused me.
>
> Are you saying that you think this code is unnecessary?
> Yes, you are right.
>
Exactly.
> It is indeed inconsistent to set zone_start to zero but not to
> set dev_start to zero.  It is the sort of thing that is probably
> worth fixing (I'd be inclined to set them both, it makes the code
> clearer) if you are making other changes to code in that area,
> but probably isn't worth fixing just by itself.
>
Even I felt that its inconistent. But felt a bit aggressive to report so.

Yes, I am looking forward to some changes but may be after the recent
patches go through, its otherwise difficult to sync.

> NeilBrown
>
>
>



-- 
Regards,
Sandeep.





 	
“To learn is to change. Education is a process that changes the learner.”
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RAID0 - unitialised zone->dev_start for zone[0]

[Bill Davidsen]

NeilBrown wrote:
> On Thu, May 14, 2009 3:08 am, SandeepKsinha wrote:
>   
>> Hi Neil,
>>
>>
>> [22:33:46 sinhas]$ diff raid0.c raid0.c.orig
>>     
>
> Always, always, always use "diff -u".  It provides context
> so the diff is much easier to read.
>
>   
>> 160d159
>> <       zone->dev_start = 0;
>> [/usr/src/linux-2.6.29/drivers/md]
>> [22:33:59 sinhas]$
>>
>> I see this piece of code missing and being used later.
>> Not very sure of the code flow, but still as part of defensive
>> programming.
>>
>> Am i missing something?
>>     
>
> conf->strip_zone is allocated with kzalloc, so all values are
> initialised to 0.
>   

To prevent future unnecessary analysis of this section, perhaps a 
comment instead of code would be appropriate?


-- 
bill davidsen <davidsen@tmr.com>
  CTO TMR Associates, Inc

"You are disgraced professional losers. And by the way, give us our money back."
    - Representative Earl Pomeroy,  Democrat of North Dakota
on the A.I.G. executives who were paid bonuses  after a federal bailout.