Thread (13 messages) 13 messages, 4 authors, 2008-05-05

Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code

From: Josh Boyer <hidden>
Date: 2008-04-21 13:08:40

On Sun, 20 Apr 2008 20:56:20 -0700
"Stephen Neuendorffer" [off-list ref] wrote:
quoted
quoted
+void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
+{
+	if (host.type == NATIVE)
+		dcr_unmap_native(host.host.native, dcr_c);
+	else
+		dcr_unmap_mmio(host.host.mmio, dcr_c);
What happens if host.type == INVALID?  Same question for the other
accessors in dcr_*_generic.
I guess looking back on it, I assumed that MAP_OK would return 0, meaning that behavior was undefined,
but I agree it's probably safer to have some error reporting there...  There starts to become a speed tradeoff
at some point, which would make function pointers more attractive.  If the ioremap does fail, or the
dcr-access-method can't be determined, then dcr_unmap_mmio would probably SEGV anyway, although that's
not something I'd really want to rely on.  I'll put an error case in there.
Well, MAP_OK would return 0, and the caller of the original dcr_map
should probably return an error or something.  But there's nothing to
enforce that.  Which is true for the current code today as well, so
it's not something you've introduced.

I just thought that if you were going to go to the trouble of defining
an invalid host type, that you'd check for it in the accessor
functions.  There is the concern of the speed tradeoffs.  I suppose you
could just omit INVALID altogether and have it match the existing code
in behavior if it was too big of a deal.

Ben, any thoughts?

josh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help