WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ppc-devel

Re: [XenPPC] [Patch] Find serial device and platform type


On Apr 26, 2006, at 3:00 PM, Maria Butrico wrote:

+    struct {
[...]
+    } serial_port;

this struct is valid outside of the structure you are defining here so please define it elsewhere. You might also want to include other members that are in struct ns16550_defaults.
Probably so, but would not this be a good time to rename this struct (ns16550) to some serial_defaults or similar?
NIT: I'm just saying it could be more useful to create a proper stuct rather than an anonymous struct within a struct.



@@ -152,6 +182,7 @@ static int __init of_getprop(int ph, con
      if (rets[0] == OF_FAILURE) {
         DBG("getprop 0x%x %s -> FAILURE\n", ph, name);
+        if (buflen > 0) memset(buf, 0, 1);
         return OF_FAILURE;
     }

hmm, I think we need to be able to depend on the interface not messing with anything on failure. I'm not sure this would protect you from anything, particularly if the property is expected to be an non-string.
The callers need to check for failure.
Yes the caller need to check for failure, but there was code in boot_of that goes something like

string[0] = '\0'
of-getprotp(string)

the string is set by get prop unless the call fails. This simplify the caller. Of course one must take care of empty string returned as well (not in the patch you see, but the in next one if I remember), but again I prefer to simply the caller.

These funtions should simply be C callable wrappers to OF methods.
The caller should be able to assume that if the call fails the buffer is untouched.


+{
+    int of_root;
+    u32 cell = 1;
+    +    of_root = of_finddevice("/");
+    if (OF_FAILURE != of_root)
if this fails, you panic()
Is this a statement, a suggestion or a problem?
suggetion.. if you can't find "/" you are so hosed.

-JX


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel