Messages in this thread | | | Date | Fri, 14 Oct 2005 03:28:32 +0200 | From | Jesper Juhl <> | Subject | Re: Fwd: Telecom Clock Driver for MPCBL0010 ATCA computer blade |
| |
On 10/14/05, Greg KH <greg@kroah.com> wrote: > On Fri, Oct 14, 2005 at 12:47:21AM +0200, Jesper Juhl wrote: > > Ick, no, that's a mess. Stick with the original version. > > > >Don't call functions within a if() statement, it's harder to read. > direct to me only? What about everyone on the list? > Whoops, wrong button - sorry. Let's copy LKML.
> > I guess you are right - it /would/ save a variable though ;) > > Not worth it. Ease of maintainability, which means being able to read > the code better, trumps a single variable on a slow path. >
Hmm, yeah, I can see the sense in that - if it's not performance critical, better make it readable.
> > > > + unsigned long tmp; > > > > + unsigned char val; > > > > + unsigned long flags; > > > > + > > > > + sscanf(buf, "%lX", &tmp); > > > > + dev_dbg(d, "tmp = 0x%lX\n", tmp); > > > > + > > > > + val = (unsigned char)tmp; > > > > > > > > You do this a lot, I'm wondering why you don't read directly into > > > > "val" and then get rid of the "tmp" variable? > > > > > > Because you want to cast it. > > > > > Ok, I'm feeling a little dense tonight, so bear with me please, but > > wouldn't the effect of reading into the unsigned char (and potentially > > getting the value truncated) result in the same thing as casting it > > later? > > I don't think it would be the same if you put a large value in the > string. Try it out and see. > Ok, I must admit I haven't actually tested it, perhaps I will tomorrow after I get some sleep.
-- Jesper Juhl <jesper.juhl@gmail.com> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |