[U-Boot-Users] [PATCH] PCI_READ_VIA_DWORD_OP: initialize val parameter on fail

Shinya Kuribayashi shinya.kuribayashi at necel.com
Wed Jun 6 02:51:49 CEST 2007


Wolfgang Denk wrote:
> In message <4665122B.6010707 at 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




More information about the U-Boot mailing list