linux-kernel - Re: [patch/rfc 2/4] pcf875x I2C GPIO expander driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 4 Apr 2008 13:18:14 -0700 (PDT)
From:	Trent Piepho <tpiepho@...escale.com>
To:	Jean Delvare <khali@...ux-fr.org>
cc:	David Brownell <david-b@...bell.net>,
	Linux Kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [patch/rfc 2/4] pcf875x I2C GPIO expander driver

On Fri, 4 Apr 2008, Jean Delvare wrote:
> On Fri, 4 Apr 2008 12:07:12 -0700 (PDT), Trent Piepho wrote:
>> On Fri, 4 Apr 2008, Jean Delvare wrote:
>>> On Thu, 3 Apr 2008 19:06:27 -0700 (PDT), Trent Piepho wrote:
>>>> +	strcpy(buf, test_bit(FLAG_IS_OUT, &gdesc->flags) ? "out\n" : "in\n\0");
>>>> +
>>>> +	return 5;
>>>
>>> Confusing construct... I suggest using sprintf instead, which will
>>> automatically return the correct number of bytes for you.
>>
>> But it's less efficient!  Will nobody think of the wasted cycles?
>
> Can you prove that it is actually less efficient, and if so, by how
> much? The time spent in this single function if probably insignificant

I think sprintf will parse the format string, and then end up calling the same
strcpy() call to handle a %s.  Since sprintf() contains the strcpy(), it has
to be slower than strcpy alone.

But I should have put a :) in there, as I changed this code.

> thread, you aren't support to include trailing \0s in the buffer you
> pass back to sysfs. Not all programming languages use \0 for string
> termination.

I fixed all those.  I wasn't clear on that when I wrote this code and forgot I
had done it incorrectly.

>>>> +	/* FIXME:  Code to remove all the sysfs devices and files created
>>>> +	 * should go here */
>>>
>>> Oh yes it really should ;)
>>
>> I know, but I'm not using modules for the system this is in, so it will never
>> get called.  What's the point of writing code I'll never use if this isn't
>> useful for the kernel?
>
> Because most certainly your code won't be accepted upstream until this
> is fixed, and presumably you posted this patch in the hope that it
> would go upstream ;) Just because it isn't useful to you doesn't mean
> it won't be useful to others. Otherwise this particular piece of code
> couldn't be built as a module at all.

I guess I was waiting for a "this could go upstream if you fix this" or "this
won't go upstream even if you fix it" so I don't waste time writing code no
one is interested in.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/