[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
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
|
