From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
Date: Wed, 06 Jun 2007 09:51:49 +0900
Subject: [U-Boot-Users] [PATCH] PCI_READ_VIA_DWORD_OP: initialize val
 parameter on fail
In-Reply-To: <20070605160404.AA899353A97@atlas.denx.de>
References: <20070605160404.AA899353A97@atlas.denx.de>
Message-ID: <46660525.3000708@necel.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Wolfgang Denk wrote:
> In message <4665122B.6010707@necel.com> you wrote:
>> pci_hose_read_config_{byte,word}_via_dword uses a temporary read
>> buffer `val32', so if read_config_dword returns -1 then val32 also
>> should be initialized.
> 
> This is actually misleading, since you  don't  initialize  the  local
> variable  "val32"  (which would not ake sense as it goes out of scope
> anyway when you return from  the  function).  Instead,  you  use  the
> variable  (more exatly, the pointer), the name of which was passed as
> a macro argument.

Ah, my bad. What should be initialized is _val_, not val32.

> I know that this is not your invention, and you  just  copy  existing
> code,  but  I  think  injecting  C statements like this through macro
> arguments is bad style and should be avoided. 
> 
> I have to admit that I don't get wht you need all this "error_code"
> trickery instead of simpley writing
> 
> 	*val = -1;
> 
> 
> Can you please change your patch like that, and while you are at it
> please fix the other uses of this ugly construct as well?

Agreed completely. I'll send patches incorporating your comments, and
with Signed-off-by.

Thanks for your review.
Shinya