On Wed, Aug 8, 2012 at 6:16 PM, York Sun <[email protected]> wrote: > On Wed, 2012-08-08 at 15:09 -0500, Andy Fleming wrote: >> Please copy me on any 85xx U-Boot patches. >> >> >> > >> > +#define HWCONFIG_BUFFER_SIZE 128 >> >> >> [...] >> >> > + char buffer[HWCONFIG_BUFFER_SIZE]; >> > + char *buf = NULL; >> > + >> > + if (getenv_f("hwconfig", buffer, sizeof(buffer)) > 0) >> > + buf = buffer; >> >> >> This seems fragile. I suppose there's no way to ensure that this >> succeeds, but we should at least check to see if the buffer we pass in >> was big enough, and let the user know that we were looking for the >> A011 erratum setting, and may not have found it because the hwconfig >> string was more than 128 characters. >> > > If the buffer isn't big enough, getenv_f() will print a message saying > that. Is that enough? Or do you prefer another warning when fsl_cpu_a011 > is not detected? It seems a little bit redundant.
Well, I think that the user will want to know that it's fsl_cpu_a011 that isn't being found. The code is able to know that, whereas the user would have to dig around for a while to figure out what was wrong. Especially if the user adds random stuff to the beginning, and that causes the erratum variable to fall off the end. Andy _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
