Patchwork [v3] ata: libata-core: initialize native_sectors in ata_hpa_resize()

login
register
mail settings
Submitter Simon Que
Date Jan. 24, 2013, 7:57 p.m.
Message ID <1359057478-3920-1-git-send-email-sque@chromium.org>
Download mbox | patch
Permalink /patch/215499/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Simon Que - Jan. 24, 2013, 7:57 p.m.
Eliminates a compiler warning about uninitialized variable.
"warning: 'native_sectors' may be used uninitialized in this function"

Signed-off-by: Simon Que <sque@chromium.org>
---
 drivers/ata/libata-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Sergei Shtylyov - Jan. 25, 2013, 11:21 a.m.
Hello.

On 24-01-2013 23:57, Simon Que wrote:

> Eliminates a compiler warning about uninitialized variable.
> "warning: 'native_sectors' may be used uninitialized in this function"

> Signed-off-by: Simon Que <sque@chromium.org>
> ---
>   drivers/ata/libata-core.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index d2b18ea..b185db1 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -1324,7 +1324,7 @@ static int ata_hpa_resize(struct ata_device *dev)
>   	int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
>   	bool unlock_hpa = ata_ignore_hpa || dev->flags & ATA_DFLAG_UNLOCK_HPA;
>   	u64 sectors = ata_id_n_sectors(dev->id);
> -	u64 native_sectors;
> +	u64 native_sectors = 0;

    Isn't it wiser to set it to 'sectors'? And frankly speaking I don't see 
how this variable may indeed be used unitialized. What version of gcc are you 
using?

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Que - Jan. 25, 2013, 11:31 p.m.
I'm using a ChromeOS / ChromiumOS gcc.

Technically it won't be used uninitialized since there's a check for
the return value.  But this isn't always obvious to the compiler --
it'd have to be smart enough to look at ata_read_native_max_address()
to see that if '*max_sectors' is not set, then there is a nonzero
return value, which would cause ata_hpa_resize() to return early.

This patch is really meant to satisfy the compiler, rather than to
catch some corner case where 'native_sectors' gets used uninitialized.
 Setting it to 0 makes more sense because its "true initialization" is
in a later call.

On Fri, Jan 25, 2013 at 3:21 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
>
> On 24-01-2013 23:57, Simon Que wrote:
>
>> Eliminates a compiler warning about uninitialized variable.
>> "warning: 'native_sectors' may be used uninitialized in this function"
>
>
>> Signed-off-by: Simon Que <sque@chromium.org>
>> ---
>>   drivers/ata/libata-core.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index d2b18ea..b185db1 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -1324,7 +1324,7 @@ static int ata_hpa_resize(struct ata_device *dev)
>>         int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
>>         bool unlock_hpa = ata_ignore_hpa || dev->flags &
>> ATA_DFLAG_UNLOCK_HPA;
>>         u64 sectors = ata_id_n_sectors(dev->id);
>> -       u64 native_sectors;
>> +       u64 native_sectors = 0;
>
>
>    Isn't it wiser to set it to 'sectors'? And frankly speaking I don't see
> how this variable may indeed be used unitialized. What version of gcc are
> you using?
>
> MBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Que - Jan. 28, 2013, 8:39 p.m.
More specifically... I have gcc 4.7.1.

On Fri, Jan 25, 2013 at 3:31 PM, Simon Que <sque@chromium.org> wrote:
> I'm using a ChromeOS / ChromiumOS gcc.
>
> Technically it won't be used uninitialized since there's a check for
> the return value.  But this isn't always obvious to the compiler --
> it'd have to be smart enough to look at ata_read_native_max_address()
> to see that if '*max_sectors' is not set, then there is a nonzero
> return value, which would cause ata_hpa_resize() to return early.
>
> This patch is really meant to satisfy the compiler, rather than to
> catch some corner case where 'native_sectors' gets used uninitialized.
>  Setting it to 0 makes more sense because its "true initialization" is
> in a later call.
>
> On Fri, Jan 25, 2013 at 3:21 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
>> Hello.
>>
>>
>> On 24-01-2013 23:57, Simon Que wrote:
>>
>>> Eliminates a compiler warning about uninitialized variable.
>>> "warning: 'native_sectors' may be used uninitialized in this function"
>>
>>
>>> Signed-off-by: Simon Que <sque@chromium.org>
>>> ---
>>>   drivers/ata/libata-core.c |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>>
>>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>>> index d2b18ea..b185db1 100644
>>> --- a/drivers/ata/libata-core.c
>>> +++ b/drivers/ata/libata-core.c
>>> @@ -1324,7 +1324,7 @@ static int ata_hpa_resize(struct ata_device *dev)
>>>         int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
>>>         bool unlock_hpa = ata_ignore_hpa || dev->flags &
>>> ATA_DFLAG_UNLOCK_HPA;
>>>         u64 sectors = ata_id_n_sectors(dev->id);
>>> -       u64 native_sectors;
>>> +       u64 native_sectors = 0;
>>
>>
>>    Isn't it wiser to set it to 'sectors'? And frankly speaking I don't see
>> how this variable may indeed be used unitialized. What version of gcc are
>> you using?
>>
>> MBR, Sergei
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d2b18ea..b185db1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1324,7 +1324,7 @@  static int ata_hpa_resize(struct ata_device *dev)
 	int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
 	bool unlock_hpa = ata_ignore_hpa || dev->flags & ATA_DFLAG_UNLOCK_HPA;
 	u64 sectors = ata_id_n_sectors(dev->id);
-	u64 native_sectors;
+	u64 native_sectors = 0;
 	int rc;
 
 	/* do we need to do it? */