Page 1 of 1

enum to string conversion issue

Posted: Thu Sep 29, 2011 2:18 pm
by Caleb1994
I was writing up the enums for PCI class codes, and realized that there wasn't going to be an easy way to extract a name, since they aren't in numerical order, so I did this:

Code: Select all

#define X(name, value) name = value,
enum pci_classcode
{
#     include "pci_classcode.h"
}
#undef X
#define X(name, value) case name: return #name; break;
static inline const char* pci_get_class_name(enum pci_classcode code)
{
     switch(code){
#          include "pci_classcode.h"
     default:
          return "Unknown PCI device class."
     }
}
and pci_classcode is filled with things like this:

Code: Select all

X(PCI_CLASS_NOTVGADEV, 0x00000000)
X(PCI_CLASS_VGACOMPATDEV, 0x00010000)
...
I have checked the output of just running cpp on my pci.h header file, and the inline function expands correctly, but I'm still getting "Unknown PCI device class" when running "puts(pci_get_class_name(PCI_CLASS_NETWORKCONTROL))".

This should work, but it's not. Any ideas?

Re: enum to string conversion issue

Posted: Thu Sep 29, 2011 3:35 pm
by Velko
Funny thing is: it does work.

Apart from few missing semicolons and undefined PCI_CLASS_NETWORKCONTROL (in your example pci_classcode.h) the code compiles and runs fine.

Re: enum to string conversion issue

Posted: Thu Sep 29, 2011 4:17 pm
by Caleb1994

Code: Select all

Funny thing is: it does work.

Apart from few missing semicolons and undefined PCI_CLASS_NETWORKCONTROL (in your example pci_classcode.h) the code compiles and runs fine.
PCI_CLASS_NETWORKCONTROL is defined in my copy of pci_classcode.h. It's kinda long and repetitive (actually not finished I don't think), but here it is:

Code: Select all

X(PCI_CLASS_NOTVGA,0x000000)
X(PCI_CLASS_VGACOMPATDEV,0x000100)
X(PCI_CLASS_SCSIBUSCONTROL,0x00010000)
X(PCI_CLASS_IDECONTROL,0x00010100)
X(PCI_CLASS_FLOPPYCONTROL,0x00010200)
X(PCI_CLASS_IPIBUSCONTROL,0x00010300)
X(PCI_CLASS_RAIDCONTROL,0x00010400)
X(PCI_CLASS_ATACONTROLSINGLEDMA,0x00010520)
X(PCI_CLASS_ATACONTROLCHAINDMA,0x00010530)
X(PCI_CLASS_SERIALATA,0x00010600)
X(PCI_CLASS_MASSSTORAGECONTROL,0x00018000)
X(PCI_CLASS_ETHCONTROL,0x00020000)
X(PCI_CLASS_TOKENRINGCONTROL,0x00020100)
X(PCI_CLASS_FDDICONTROL,0x00020200)
X(PCI_CLASS_ATMCONTROL,0x00020300)
X(PCI_CLASS_ISDNCONTROL,0x00020400)
X(PCI_CLASS_WORLDFIPCONTROL,0x00020500)
X(PCI_CLASS_PICMG214,0x00020600)
X(PCI_CLASS_NETWORKCONTROL,0x00020800)
X(PCI_CLASS_VGACOMPATCONTROL,0x00030000)
X(PCI_CLASS_8512CONTROL,0x00030001)
X(PCI_CLASS_XGACONTROL,0x00030100)
X(PCI_CLASS_3DCONTROL,0x00030200)
X(PCI_CLASS_OTHERDISPLAYCONTROL,0x00038000)
X(PCI_CLASS_VIDEODEVICE,0x00040000)
X(PCI_CLASS_AUDIODEVICE,0x00040100)
X(PCI_CLASS_TELEPHONYDEVICE,0x00040200)
X(PCI_CLASS_MULTIMEDIADEVICE,0x00048000)
X(PCI_CLASS_RAMCONTROL,0x00050000)
X(PCI_CLASS_FLASHCONTROL,0x00050100)
X(PCI_CLASS_MEMORYCONTROL,0x00058000)
X(PCI_CLASS_HOSTBRIDGE,0x00060000)
X(PCI_CLASS_ISABRIDGE,0x00060100)
X(PCI_CLASS_EISABRIDGE,0x00060200)
X(PCI_CLASS_MCABRIDGE,0x00060300)
X(PCI_CLASS_PCIPCIBRIDGE,0x00060400)
X(PCI_CLASS_PCIPCIBRIDGE_SUBDECODE,0x00060401)
X(PCI_CLASS_PCMCIABRIDGE,0x00060500)
X(PCI_CLASS_NUBUSBRIDGE,0x00060600)
X(PCI_CLASS_CARDBUS_BRIDGE,0x00060700)
X(PCI_CLASS_RACEWAYBRIDGE,0x00060800)
X(PCI_CLASS_PCIPCIBRIDGE_PRIMTRANS,0x00060940)
X(PCI_CLASS_PCIPCIBRIDGE_SECONDTRANS,0x00060980)
X(PCI_CLASS_INFINIBRANDPCIHOSTBRIDGE,0x00060A00)
X(PCI_CLASS_OTHERBRIDGEDEVICE,0x00068000)
X(PCI_CLASS_GENERICXTSERIALCONTROL,0x00070000)
X(PCI_CLASS_16450COMPATCONTROL,0x00070001)
X(PCI_CLASS_16550COMPATCONTROL,0x00070002)
X(PCI_CLASS_16650COMPATCONTROL,0x00070003)
X(PCI_CLASS_16750COMPATCONTROL,0x00070004)
X(PCI_CLASS_16850COMPATCONTROL,0x00070005)
X(PCI_CLASS_16950COMPATCONTROL,0x00070006)
X(PCI_CLASS_PARALLELPORT,0x00070100)
X(PCI_CLASS_BIDIRPARALLELPORT,0x00070101)
X(PCI_CLASS_ECP1XPARELLELPORT,0x00070102)
X(PCI_CLASS_IEEE1284CONTROL,0x00070103)
X(PCI_CLASS_IEEE1284TARGETDEVICE,0x00070104)
This is very strange... I typed that source snippet from the last post in from memory without looking at my source (hence the missing semicolon), but the one in my source didn't work, but this one does. Whatever. I don't care. lol I copied the one out of the post, and dropped it in my PCI header and it worked. Yay for strange bugs that seemingly fix themselves (but not all the time. I'd get bored. :) )


Oh! And Burkus:

I know I don't NEED the break, but the compiler should optimize it out (even if it doesnt, whats one tiny jump op in the grand scheme of things?), and I like everything to be closed off. That way 1. it looks nice, and 2. It's a habbit. No matter what I ALWAYS put a break after a case statement (had some pesky bugs from that along the way :P)

Edit:

Yes, anyone who notices. I did put the Revision ID null byte in those class code definitions in the wrong place. I have moved them in my code to the first byte. (that way I can just read the long at 08h and null the low byte. I don't have to deal with shifting to find the class)

Re: enum to string conversion issue

Posted: Thu Sep 29, 2011 9:37 pm
by Brendan
Hi,

For "Unknown PCI device class", just return NULL. That way the caller can check for NULL and display more information itself. For example, it could display "Unknown PCI device class (class code was 0x1234)" so that you know which class code your software is missing.

The next step is to recognise that the "class code" field is actually 3 separate sub-fields - a "base class", a "sub-class" and a "programming interface". This lends itself more to something using "nested switch", like:

Code: Select all

    switch(code >> 16) {
        case 0x00:
            return "Unknown device (too old for class code)";
        case 0x01:
            switch( (code >> 8) & 0xFF) {
                case 0x00:
                    return "SCSI controller";
                case 0x01:
                    return "IDE controller";
                case 0x05:
                    switch(code & 0xFF) {
                        case 0x20:
                            return "ATA controller (single DMA)";
                        case 0x30:
                            return "ATA controller (double DMA)";
                        default:
                            return "Unknown ATA controller";
                    }
                default:
                    return "Unknown mass storage controller";
            }
        default:
            return "Unknown device (unknown class code)";
That way, if you know it's a mass storage controller (base class = 0x01) then you can say so even though you don't recognise the sub-class or interface; and if you know it's an ATA controller (base class = 0x01, sub-class = 0x05) then you can say so even though you don't recognise the interface.

For simplicity, you could probably search a flattened list containing "code, number_of_components, string" entries to find the entry that highest number of matching components. For example:

Code: Select all

myArrayofCodes[] = {
    {0x000000, 1, "Unknown class code (too old)"},
    {0x010000, 1, "Unknown mass storage device"},
    {0x010000, 2, "SCSI controller"},
    {0x010100, 2, "IDE controller"},
    {0x010500, 2, "ATA controller"},
    {0x010520, 3, "ATA controller (single DMA)"},
    {0x010530, 3, "ATA controller (double DMA)"},

Code: Select all

    bestString = "Unknown class code";
    bestComponents = -1;
    
    for(i = 0; i < MAX; i++) {
        if(myArrayofCodes[i].components > bestComponents) {
            switch(myArrayofCodes[i].components) {
            case 1;
                if( (myArrayofCodes[i].code & 0xFF0000) == (code & 0xFF0000) {
                    bestString = myArrayofCodes[i].string;
                    bestComponents = 1;
                }
                break;
            case 2;
                if( (myArrayofCodes[i].code & 0xFFFF00) == (code & 0xFFFF00) {
                    bestString = myArrayofCodes[i].string;
                    bestComponents = 2;
                }
                break;
            case 3;
                if(myArrayofCodes[i].code == code) {
                    return myArrayofCodes[i].string;
                }
            }
        }
    }

Cheers,

Brendan