Dear Reinhard Meyer,

In message <4c87cfe8.8010...@emk-elektronik.de> you wrote:
> Dear Mike Frysinger,
> > On Monday, September 06, 2010 10:59:16 Reinhard Meyer wrote:
> >> +int enc_miiphy_read(const char *devname, u8 phy_adr, u8 reg, u16 *value)
> >> +{
> >> +  struct eth_device *dev = eth_get_dev_by_name(devname);
> >> +  if (dev) {
> >> +          enc_dev_t *enc = to_enc(dev);
> >> +
> >> +          if (phy_adr != 0)
> >> +                  return -1;
> >> +          spi_claim_bus(enc->slave);
> >> +          *value = phy_read(enc, reg);
> >> +          spi_release_bus(enc->slave);
> >> +          return 0;
> >> +  }
> >> +  return -1;
> >> +}
> >
> > seems like this (and the write func) would be nicer if it were:
> > if (!dev)
> >     return -1;
> > ...
> 
> Umm, no. That would split enc_dev_t *enc = to_enc (dev) into two lines:
> Declaration, if, assigment

Yes? What's the problem with that?  But then you can unindent a whole
block of code one level, which means the code is much easier toi read
and to maintain.

Please do as Mike suggested.

> > check the return value
> 
> OK, for sake of completeness in every function using it..
> Hmm blows up the code :)

Yes, error handling blows up the code. But it helpos a lot to make
programs reliable and robust.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
There has been an alarming increase in the number of things you  know
nothing about.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to