pci.c z.155 die rückgabe von pci_config_read() ist doch ein uint32_t?



  • btw. was macht eigentlich der usb und rtl8139 code in der pci scan function? den könnte man doch evtl. in die entsprechenden treiber module auslagern?

    axo ja wäre es nicht besser das pci struct in eine form zu bringen, so dass man es leichter lesen könnte, also ich meine so wie es von der hardware angeboten wird 64Byte Basis-Header -> 64Byte Software-Struct?

    also ich hoffe ich nerv euch nicht mit meinem scheiß, wenns euch stört einfach bescheid geben 🙂

    lg lolo

    @edit hach wie ist das schön, wenn man seine tipp fehler wieder editieren kann 😃



  • uint32_t pci_config_read( uint8_t bus, uint8_t device, uint8_t func, uint16_t content );
    
    uint16_t vendorID = pci_config_read(bus, device, func, PCI_VENDOR_ID);
    

    Tatsächlich. 😮 Offenbar funktioniert "-Wall" und "-Werror" beim gcc hier nicht so ganz.


  • Mod

    PCI_VENDOR_ID bedeutet, dass nur zwei Byte zurück gegeben werden, daher ist uint16_t richtig, aber ein interessanter Hinweis. 🙂



  • Aber pci_config_read liefert immer einen "geandeten" uint32_t zurück:

    // pci.c rev 391
    
    uint32_t pci_config_read(uint8_t bus, uint8_t device, uint8_t func, uint16_t content)
    {
    (...)
     uint32_t readVal = inportl(PCI_CONFIGURATION_DATA) >> (8 * offset);
    (...)
     return readVal;
    }
    

    Das hätte Kompilerwarnungen geben müssen. 😕



  • tja, gcc hat halt Macken, die einen sagen mehr (ich auch^^), die anderen weniger.



  • Nix da, der Compiler ist perfekt. Für mehr Warnungen: http://www.c-plusplus.net/forum/viewtopic-var-t-is-256511-and-highlight-is-strengste.html

    -Wconversion müsste der richtige Schalter sein.

    Ansonsten wäre wieder die Frage nach der statischen Codeanalyse, sprich, ob splint oder nicht... 🙂



  • Gibts nicht vlt. irgendeine Option, die alle Warnungen aktiviert?



  • Nein, es gibt keine.



  • Folglich gilt beim gcc: "-Wall" != "-Wall". 🙂

    Aber die Sache mit dem Rückgabewert ist weniger lustig.



  • Die wollt ihr vermutlich auch gar nicht, bei den lustigen Dingern, die es da so gibt (z. B. signed-unsigned-Warnungen). -pedantic dürfte aber auch noch mal ein paar Sachen aufdecken (wobei das nicht unbedingt sinnvoll sein muss).



  • tatsächlich, pedantic ist nicht drin in unseren Flags... Danke für den Hinweis, XanClic.

    Vlt sollten wir über einen anderen Compiler mal nachdenken (Clang vlt.). Ich werd da mal rumprobieren...



  • Mr X schrieb:

    Vlt sollten wir über einen anderen Compiler mal nachdenken (Clang vlt.). Ich werd da mal rumprobieren...

    Das wird schwierig, wegen Inline-Assembler 🙂



  • Das fürchte ich auch...



  • pedantic geht nicht... Er warnt zwar jetzt vor mehr sachen (u.a. Konversion von Fkt-Ptr. nach void*), aber dafür warnt er auch vor GCC-Extensions (ein weiteres Portierungshindernis, btw.), die unser ehci-Treiber nutzt.

    EDIT: Und Wconversion warnt tatsächlich vor der Konversation mit Datenverlustgefahr...



  • also mein spontaner gedanke war, dass der pci_config_read() ohne das verunde im unteren teil immer ein uint32_t hergibt. das sollte von der performance auch nicht schlechter sein. also wieso nicht im ersten schritt gleich vendorId und deviceId einlesen (in ein struct) und dann auf if(myPciStruct.vendorId == 0xFFFF) testen dann müßte man im fall das man ein pci-device gefunden hat nicht nochmal deviceId einlesen. wenn man keins gefunden hat dann hat man es zwar umsonst eingelesen, aber das dürfte performance mäßig nicht schlechter sein da ja der bus eh schon die 32bit hergibt.

    lg lolo



  • Mr X schrieb:

    EDIT: Und Wconversion warnt tatsächlich vor der Konversation mit Datenverlustgefahr...

    Bei solchen Stellen im Code ist es interessant, warum die Software funktioniert...
    Mit splint könnte man noch weitere Fehler aufdecken, irgendwo habe ich mal gehört, 1 Fehler pro 6 Codezeilen o.ä. - unglaublich, aber wahr 😉



  • abc.w schrieb:

    1 Fehler pro 6 Codezeilen o.ä. - unglaublich, aber wahr...

    die faustregel lautet:

    1/ksloc bei komerzieller software
    0.3/ksloc bei open source

    😉

    lg lolo



  • noob_lolo schrieb:

    1/ksloc bei komerzieller software
    0.3/ksloc bei open source

    Kann sein... aber wo gibt es dazu Statistiken?



  • abc.w schrieb:

    noob_lolo schrieb:

    1/ksloc bei komerzieller software
    0.3/ksloc bei open source

    Kann sein... aber wo gibt es dazu Statistiken?

    PS: Ne, kann nicht sein, das ist viel zu optimistisch 🙂 Wäre aber natürlich schön...



  • Ich hab das grad mal ausprobiert mit Splint und das kommt gar nicht durch den Code durch. Es stößt sich erst an den va_args builtins von gcc. Danach scheint es an jeder Stelle, wo "void* variablenname = ..." nicht am Anfang einer Funktion steht, mit einem parse error zu stoppen.


Anmelden zum Antworten