Re: libfdt: going forward for u-boot

[Jerry Van Baren <gerald.vanbaren@comcast.net>]

David Gibson wrote:
> On Wed, Mar 21, 2007 at 09:12:31PM -0400, Jerry Van Baren wrote:
>> David Gibson wrote:

[snip]

>>>> fdt_rw.c
>>>> --------
>>>> * Nonessential improvements to fdt_open_into()
>>>>   * Validate the header (fdt_move() does the validation, but I skip
>>>>       the move if buf == fdt so I needed to add the validation)
>>>>   * Do the fdt_move() only if (buf != fdt) - I call this at times with
>>>>       a new (longer) length but don't want to actually move the blob,
>>>>       just want the longer length so I can modify the fdt in-place.  The
>>>>       current code works, but it seemed silly to do a move if I'm not
>>>>       actually moving it.
>>> If fdt == buf, the memmove() in fdt_move() will become a no-op in any
>>> case.  I don't see any point to conditionalizing the fdt_move().
>> But it allows me to make the blob bigger.  Ahh, it looks like all I need 
>> to do is:
>>          fdt_set_header(fdt, totalsize, bufsize);
>> so I agree, I'm working way too hard on this one.
> 
> No, if you want to make more space to do edits in, you should
> definitely be using fdt_open_into() - that's the purpose of
> fdt_open_into().  I'm just saying there's no need to conditionalize
> the fdt_move(), because fdt_move() *already* correctly handles the
> case where fdt==buf.

Not in my copy unless the memmove() implementation counts.
--------------------------------
int fdt_move(const void *fdt, void *buf, int bufsize)
{
         int err = _fdt_check_header(fdt);

         if (err)
                 return err;

         if (fdt_totalsize(fdt) > bufsize)
                 return -FDT_ERR_NOSPACE;

         memmove(buf, fdt, fdt_totalsize(fdt));
         return 0;
}
--------------------------------
--- a/fdt_rw.c
+++ b/fdt_rw.c
@@ -260,11 +260,19 @@ int fdt_del_node(void *fdt, int nodeoffset)

  int fdt_open_into(void *fdt, void *buf, int bufsize)
  {
-       int err;
-
-       err = fdt_move(fdt, buf, bufsize);
-       if (err)
-               return err;
+        int err = rw_check_header(fdt);
+
+        if (err)
+                return err;
+
+       /*
+        * If we are just expanding the size in-place, no need to do the 
move.
+        */
+       if (buf != fdt) {
+               err = fdt_move(fdt, buf, bufsize);
+               if (err)
+                       return err;
+       }

         fdt = buf;

--------------------------------
(Yes, the above is whitespace damaged.  It ain't a patch ;-)

Best regards,
gvb