Jaderné noviny - 18. 2. 2016: Interface pro načítání souborů v jádru (diskuse)

[Jardík]

Který pako zase navrhává API s debilním typem určující velikost bufferu

int kernel_read_file(struct file *file, void **buf, loff_t *size,
                     loff_t max_size, enum kernel_read_file_id id);

Kdyby to byl nějaký offset v souboru, tak ani nepípnu, jako že soubor může být větší než maximální velikost objektu v paměti. Ale proč pak tento typ cpát i sem. Jako následek to bude mít, že ten, kdo chce API použít, bude muset kontrolovat hodnoty typu size_t, jestli se vejdou do loff_t a pak zase opačně. Naprostá pakárna.

int kernel_read_file(struct file *file, void **buf, size_t *size,
                     size_t max_size, enum kernel_read_file_id id);

Stejnou debilotinu obsahuje např. Qt. Kde fce na načtení celého souboru bere typ qint64 i na 32bit platformě, kde to těžko může do paměti načíst... a ještě to načítá do QVectoru, který používá pro určení velikosti int. Proboha, ty lidi nepřemejšlej.

[Pali]

A prečo sa vyjadruješ tu pod týmto článkom, kde to nikto z predkladateľov patchu vôbec nesleduje namiesto toho aby si sa vyjadril priamo pri diskusii o tom patchu, na príslušnom mailingliste, kde vedie??

[Jardík]

Protože není, kde jinde se vyjádřit. Všude samé registrace a nebo mailing listy, které pak posílají haldu emailů a předávají emailové adresy spam botům.

[Pali]

Na LKML sa registrovať vôbec nemusiš. A väčšina kernelových mailinglistov je tieť otvorená pre všetkých ľudí bez registrácie. Na väčšinu mailing listov sa ani zapísať nemusíš aby si sa mohol vyjadriť, žiadna halda emailov ti nehrozí. A ak nechceš zverejniť svoju identitu verjene v podobe emailovej adresy, prečo nenapíšeš svoje pripomienky tajne konkrétnej osobe, ktorá ten patch napísala alebo osobe, ktorá ten patch začlenila? Je to až také ťažké? Ale keď sa nechceš vyjadrovať, tam kde to má zmysel na mieste na tom určenom, tak sa choď vyplakať do kúta a nie tu...

[Michal Kubeček]

A taky je zvykem odpovídat zásadně všem, takže se není ani potřeba bát, že člověk neuvidí odpovědi.

[Jardík]

Nedávno jsem např. odpovídal tajně tady. V článku pak zkomolil moje jméno a stejně pak opravil něco jiného, čímž nadělal ještě více chyb a původní problém neopravil. Poslal jsem mu pak další email, a ten už pak ignoroval a kód má stále rozbitý.

public:
    std::shared_ptr<C> add(C && callback) {
        auto shared = std::make_shared<C>(callback);
        this->callbacks.push_back(shared);
        return shared;
    }
    template <typename ...A>
    void invoke(A && ... args) {
        // Go over all callbacks and dispatch on those that are still available.
        // Remove all callbacks that are gone.
        typename std::vector<std::weak_ptr<C>>::iterator iter;
        for (iter = this->callbacks.begin(); iter != this->callbacks.end(); ) {
            auto callback = iter->lock();
            if (callback) {
                (*callback)(std::forward<A>(args)...);
                ++iter;
            } else {
                iter = this->callbacks.erase(iter);
            }
        }
    }

Všimněte si, že počítá s tím, že erase() zneplatní iterátory, takže korektně použije návratovou hodnotu jako nový iterátor. Už ale nepočítá s tím, že zavolání callbacku může mít za následek volání add() k přidání nového callbacku. Provede se push_back() a pokud dojde k realokaci úložiště vectoru, tak se mu iterátory zneplatní a v dobrém případě pak dostane SIGSEGV, v tom horším se mu smaže pevný disk. Doporučil jsem mu tedy použít indexy místo iterátorů.

Místo toho si pak vzpomněl (jeho update jako reakce na můj email, který si asi špatně vyložil), že chce podporovat rekurzivní volání invoke(), s čímž si přivodil spoustu dalších problémů. Zneplatnění iterátorů stále nevyřešil a indexy nepoužil, a je sporné, zda by rekurzivní volání invoke() mělo začínat opět od začátku či začít tam, kde předchozí skončilo, aby nedošlo k nechtěnému volání callbacků 2x, pokud následující mělo odstranit to předcházející. Dále je sporné, zda je dobré pročistit neplatné callbacky až po dokončení všech iterací a jen když nejsou už žádné rekurzivní invoke(), čímž dochází k větší a větší konzumaci paměti při rekurzivních volání a častém přidávání callbacků.

Upozorňuji že kód je citace a vztahuje se na něj původní copyright, jestli takový je.

[chrono]

Na prístup k veľkosti súboru sa používa i_size_read a tá funkcia vráti loff_t, takže max_size musí byť loff_t. size pravdepodobne mohlo byť aj size_t, autorka sa zrejme rozhodla, že bude jednoduchšie, ak budú oba parametre rovnakého typu (prípadne to má úplne inú príčinu).

[Michal Kubeček]

Vzhledem k tomu, že loff_t je typedefovaný long long a na všech platformách je 64-bitový (tedy 63+1), tak bych v tom žádnou vědu nehledal. Až chudák Jardík zjistí, na kolika místech se automaticky předpokládá sizeof(long) == sizeof(void *), tak ho z toho nejspíš klepne.

[Jardík]

To já vím, že v kernelu se předpokládá sizeof(long) == sizeof(void *). Bohužel, autorům se z nějakého důvodu nelíbí uintptr_t/intptr_t, tak se uchylují k takovým prasárnám. Přitom kernel normálně používá C99, takže nevidím důvod uintptr_t nepoužít. Je to volitelný typ, ale kdyby na platformě neexistoval, tak nebude ani bezpečné to uložit do typu long.

Problém je právě to, že long long je třeba 64bit na nějaké té 32bit platformě (resp. je garantována taková jeho minimální velikost standardem). Funkce mi pak na 32bit platformě říká "Hele, tato funkce může alokovat buffer o velikosti větší, než je maximální přípustná velikost objektu, a můžeš tedy takovou věc po mně chtít". Stejný případ jako v Qt, které tvrdí "Do vectoru s maximální kapacitou INT_MAX jsem schopný načíst soubor o velikosti až INT64_MAX". Ale jestli to tedy nějak kernel umí a vrací to tak už nějaké to původní volání, tak se tedy autorce patche omlouvám a směřuji svou výtku dále na autora původního volání.

[kvr]

Ono je to z jistého pohledu v pořádku. Caller nespecifikuje velikost paměti, ale velikost souboru, který se má načíst, caller totiž rovněž dostane velikost souboru. Proto tedy loff_t. To, že to potom spadne na nedostatečné velikosti paměti, je jiná věc, ale v podstatě jen zajistí další kontrolu konsistence.

Jinak se dá celkem tiše předpokládat, že loff_t bude aspoň stejně velký jako size_t, v tom bych problém neviděl.