FYI, I propose to pick this up for the *NEXT* release of u-boot ("Yellow
Submarine" ;-). My reasoning is that (1) the current window closes
today, which makes me uncomfortable WRT putting in changes today and (2)
this appears to be a latent bug (at worst, a misleading error message);
I have not seen evidence that it is causing problems at the moment.
Best regards,
gvb
David Gibson wrote:
> The current implementation of fdt_get_path() has a couple of bugs,
> fixed by this patch.
>
> First, contrary to its documentation, on success it returns the length
> of the node's path, rather than 0. The testcase is correspondingly
> wrong, and the patch fixes this as well.
>
> Second, in some circumstances, it will return -FDT_ERR_BADOFFSET
> instead of -FDT_ERR_NOSPACE when given insufficient buffer space.
> Specifically this happens when there is insufficient space even to
> hold the path's second last component. This behaviour is corrected,
> and the testcase updated to check it.
>
> Signed-off-by: David Gibson <[EMAIL PROTECTED]>
>
> Index: dtc/tests/get_path.c
> ===================================================================
> --- dtc.orig/tests/get_path.c 2008-08-29 14:11:09.000000000 +1000
> +++ dtc/tests/get_path.c 2008-08-29 14:11:11.000000000 +1000
> @@ -43,6 +43,8 @@ static void check_path_buf(void *fdt, co
> memset(buf, POISON, sizeof(buf)); /* poison the buffer */
>
> len = fdt_get_path(fdt, offset, buf, buflen);
> + verbose_printf("get_path() %s -> %d -> %s\n", path, offset, buf);
> +
> if (buflen <= pathlen) {
> if (len != -FDT_ERR_NOSPACE)
> FAIL("fdt_get_path([%d bytes]) returns %d with "
> @@ -51,9 +53,9 @@ static void check_path_buf(void *fdt, co
> if (len < 0)
> FAIL("fdt_get_path([%d bytes]): %s", buflen,
> fdt_strerror(len));
> - if (len != pathlen)
> - FAIL("fdt_get_path([%d bytes]) reports length %d "
> - "instead of %d", buflen, len, pathlen);
> + if (len != 0)
> + FAIL("fdt_get_path([%d bytes]) returns %d "
> + "instead of 0", buflen, len);
> if (strcmp(buf, path) != 0)
> FAIL("fdt_get_path([%d bytes]) returns \"%s\" "
> "instead of \"%s\"", buflen, buf, path);
> @@ -70,6 +72,8 @@ static void check_path(void *fdt, const
> check_path_buf(fdt, path, pathlen, 1024);
> check_path_buf(fdt, path, pathlen, pathlen+1);
> check_path_buf(fdt, path, pathlen, pathlen);
> + check_path_buf(fdt, path, pathlen, 0);
> + check_path_buf(fdt, path, pathlen, 2);
> }
>
> int main(int argc, char *argv[])
> Index: dtc/libfdt/fdt_ro.c
> ===================================================================
> --- dtc.orig/libfdt/fdt_ro.c 2008-08-29 14:11:11.000000000 +1000
> +++ dtc/libfdt/fdt_ro.c 2008-08-29 14:11:11.000000000 +1000
> @@ -328,9 +328,6 @@ int fdt_get_path(const void *fdt, int no
> for (offset = 0, depth = 0;
> (offset >= 0) && (offset <= nodeoffset);
> offset = fdt_next_node(fdt, offset, &depth)) {
> - if (pdepth < depth)
> - continue; /* overflowed buffer */
> -
> while (pdepth > depth) {
> do {
> p--;
> @@ -338,14 +335,16 @@ int fdt_get_path(const void *fdt, int no
> pdepth--;
> }
>
> - name = fdt_get_name(fdt, offset, &namelen);
> - if (!name)
> - return namelen;
> - if ((p + namelen + 1) <= buflen) {
> - memcpy(buf + p, name, namelen);
> - p += namelen;
> - buf[p++] = '/';
> - pdepth++;
> + if (pdepth >= depth) {
> + name = fdt_get_name(fdt, offset, &namelen);
> + if (!name)
> + return namelen;
> + if ((p + namelen + 1) <= buflen) {
> + memcpy(buf + p, name, namelen);
> + p += namelen;
> + buf[p++] = '/';
> + pdepth++;
> + }
> }
>
> if (offset == nodeoffset) {
> @@ -355,7 +354,7 @@ int fdt_get_path(const void *fdt, int no
> if (p > 1) /* special case so that root path is "/",
> not "" */
> p--;
> buf[p] = '\0';
> - return p;
> + return 0;
> }
> }
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
