Bug 175351 - displaced enumerated value knackers parport_pc
Summary: displaced enumerated value knackers parport_pc
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 4
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dave Jones
QA Contact: Brian Brock
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2005-12-09 08:51 UTC by JW
Modified: 2015-01-04 22:23 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-12-12 23:38:45 UTC


Attachments (Terms of Use)
Puts parport_pc mobility_pp enum in right position (957 bytes, patch)
2005-12-09 08:52 UTC, JW
no flags Details | Diff
Patch to correct mobility_pp ordering problem (1.26 KB, patch)
2005-12-09 09:05 UTC, JW
no flags Details | Diff

Description JW 2005-12-09 08:51:03 UTC
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (compatible; MSIE 6.0; Windows; U; AIIEEEE!; Win98; Windows 98; en-US; Gecko masquerading as IE; should it matter?; rv:1.8b) Gecko/20050217

Description of problem:
There is a misplaced enumerated value in parport_pc.c which causes faulty matching of vendor/device pci entries.


Version-Release number of selected component (if applicable):
kernel-2.6.14-1.1644_FC4

How reproducible:
Always

Steps to Reproduce:
1.read drivers/parport/parport_pc.c
2.
3.
  

Actual Results:  "How on earth could and enumerated value get out of whack", you will say.
Then you will say, "Why on earth doesn't the code check that enumerated values match required displacement into table so that this sort of bug just cannot happen?".


Expected Results:  You shouldn't have to say anything.


Additional info:

Comment 1 JW 2005-12-09 08:52:29 UTC
Created attachment 122071 [details]
Puts parport_pc mobility_pp enum in right position

Comment 2 JW 2005-12-09 09:05:22 UTC
Created attachment 122072 [details]
Patch to correct mobility_pp ordering problem

Please discard previous patch because there is also another table entry that
must also be reordered. Now that I have discovered another table ordering
problem it would have probably been just as easy to adjust
parport_pc_pci_tbl[].
Never mind, this patch obsoletes the previous one and seems to work ok.

Only affects people trying to use syba, titan, avlab, aks, and netmos cards.

Comment 3 Dave Jones 2005-12-10 05:51:58 UTC
whats actually the problem here? I can't figure out why moving that enum to a
different value actually fixes anything.

From the diff, it seems you're just moving it from the 43rd member to the 33rd
member.  What am I missing here ?

(You should probably post this upstream too to linux-kernel@vger.kernel.org)



Comment 4 JW 2005-12-12 07:38:57 UTC
(In reply to comment #3)
> 
> From the diff, it seems you're just moving it from the 43rd member to the 33rd
> member.  What am I missing here ?
>

You have to read the code to understand.

Look at parport_pc_pci_probe() and study the variable 'i' and how it is used.
parport_pc_pci_tbl[].driver_data is enum type, and that enum is also used as an
index into table cards[].  So cards[] must match ordering of enum.

Actually my patch is wrong, because the printk (approx line 2960) should be
using id->vendor instead of indexing into parport_pc_pci_tbl[] with driver_data.

But cards[] *must* have correct ordering.

Comment 5 Dave Jones 2005-12-12 23:38:45 UTC
I follow what you're saying with the table & enum having to be in sync, but what
I'm failing to see is how they're out of sync.

mobility_pp looks to be in the right place both before, and after your diff as
you're moving its entry in both places.

This really should be taken upstream, where people more familiar with this
driver can comment.


Comment 6 JW 2005-12-13 00:01:20 UTC
My changes did cure the problem.  I know this because I made the changes,
recompiled the modules, and tested them!

Without the patch kernel printk's would show wrong vendor:device ids.

It is all a bit more complicated than that anyhow.  My original patch wasn't the
best.  In actual fact the tables should be constructed using [enum]={...}
entries rather than the existing "/*enum*/{...}" entries.

There is also a bug where _pci_tbl[enum].vendor is used instead of id->vendor etc.

There is also bug where class of board is not checked (so that parport_serial
never gets a chance).

There are also changes needed to handle quirks for timedia serial/parallel cards
which have wrong pci class.

Surely you must have encountered these problems with your higher quality RHEL
product?  Or do you still get RHEL bugs fixed upstream?


Comment 7 Dave Jones 2005-12-13 00:12:54 UTC
if its an issue in RHEL, no-one reported it, and actually yes, we do try to get
things fixed upstream first, so that they get coverage testing before the
patches land in RHEL.



Note You need to log in before you can comment on or make changes to this bug.