linux-kernel - Re: [GIT PATCH] HID and USB HID update for 2.6.21-rc2

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 linux-cve-announce 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]

Message-ID: <Pine.LNX.4.64.0702281814380.4255@jikos.suse.cz>
Date:	Wed, 28 Feb 2007 18:25:55 +0100 (CET)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [GIT PATCH] HID and USB HID update for 2.6.21-rc2

On Wed, 28 Feb 2007, Linus Torvalds wrote:

> > The diffstat looks larger because the usbhid code is moved from 
> > USB-specific directory to HID-specific directory
> No. The diffstat looks huge because you moved "hid_blacklist" into a 
> header file, and that is a big enough change that git won't consider the 
> rename to be just a rename any more (you basically moved the old 
> usbdrivers/usb/input/hid-core.c file into *two* files: hid-core.c and 
> usbhid.h).

Yes, but was done by two separate commits, diffstat -M for 
3d5af52d0997d545995d8747c8057be5dee49b14 shows hid-core.c as a 100% 
rename, but later commit b41ea57c01a1943ab36af0017cfc1329815af9ce splits 
it, so in cummulative diffstat it doesn't show it as a rename.

> Why do that? It now gets included INTO EVERY DAMN FILE that includes 
> <usbhid.h>, since that one now has that
> 	static const struct hid_blacklist
> definition in it. Yet *nothing* wants it, except for the one C file that 
> it used to be in.

You're right that usbhid.h is not a best place for it. The thing is that 
hid_blacklist[] and related things just badly needs cleanup - it has been 
for quite a long time placed on a really random place in the middle of a 
.c file. In addition to that, the corresponding #defines are scatthered 
around, interleaved with functions (see for example where 
USB_VENDOR_ID_PANJIT and USB_VENDOR_ID_TURBOX defines are).

> I'm also not going to pull it if you just add a new commit to undo the 
> idiocy. That thing needs to be totally re-done, as far as I'm concerned. I 
> don't want to touch anything that has EVER even *seen* anything that 
> stupid.

This IMHO just needs cleanup. Will you accept creating a separate header 
file solely for purposes of this blacklist and related defines?

Otherwise I will just drop this cleanup, but I still think that the 
current situation is horrible.

> Yes, I'm grumpy. I don't like big changes at this stage, and if they are 
> also STUPID big changes, as this seems to be, I refuse to pull them 
> entirely.

Are you also opposed to just the code movement? There are some bugfixes I 
think that really need merging, so just to know what would be acceptable 
for you at the time being.

Thanks,

-- 
Jiri Kosina
-
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/