Bug 175351 - displaced enumerated value knackers parport_pc
displaced enumerated value knackers parport_pc
Status: CLOSED UPSTREAM
Product: Fedora
Classification: Fedora
Component: kernel (Show other bugs)
4
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dave Jones
Brian Brock
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2005-12-09 03:51 EST by JW
Modified: 2015-01-04 17:23 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-12-12 18:38:45 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


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

  None (edit)
Description JW 2005-12-09 03:51:03 EST
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 03:52:29 EST
Created attachment 122071 [details]
Puts parport_pc mobility_pp enum in right position
Comment 2 JW 2005-12-09 04:05:22 EST
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 00:51:58 EST
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 02:38:57 EST
(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 18:38:45 EST
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-12 19:01:20 EST
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-12 19:12:54 EST
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.