[C] variabel set check - NULL of extra var?

[Verwijderd]

Ik zit nu ff met een driver te spelen waar ik al een tijdje de maintainer van ben - ben bezig met grondig redesign van 't ding (code nog niet publiekelijk beschikbaar ). Nu heeft deze driver voor een enorm aantal variabelen een extra variabele - is_set. Het is een videokaart-achtig-iets driver, en hij heeft is_set opties voor de window, de framebuffer, de capture format, etc.

Nu vind ik dit behoorlijk lelijk, ik gebruik over het algemeen hiervoor de variabele zelf, waarbij NULL aangeeft dattie niet geset is...

Voorbeeldje:

code:

struct video_buffer {
  void *base;
[..]
};

struct zoran {
[..]
  struct video_buffer buffer;
  int buffer_set;
[..]
};

buffer_set is 1 of nul, en de buffer zit dus in zoran.buffer.base. Nu zou ik dat dus zonder die int buffer_set doen en de zoran.buffer.base op NULL of non-NULL zetten om dit te doen - dat scheelt weer vier bytes en maakt het m.i. overzichtelijker (mits gedocumenteerd).

Echter, die module is geschreven door een bijzonder goede programmeur en ik heb dan zoiets van 'zou ik als n00b [..] dat wel moeten veranderen'. Ik wil dus graag voldoen aan de standaard linux kernel coding guidelines, want de driver zit in de kernel en moet daar ook blijven. .

Weet iemand of hier een soort consensus voor is? Voorkeuren? Argumenten?

[.oisyn]

als die video_buffer alleen die pointer zou bevatten, zou ik het idd ook met NULL doen. Echter, aan die [..] te zien bevat ie meer members dan alleen base (anders was die struct ook een beetje nutteloos ), zou ik net als de goede man in kwestie een extra variabele gebruiken om te controleren of ie geset is of niet. (Mits je niet al teveel geheugenruimte tot je beschikking hebt, maar dat betwijfel ik )

ik vind het eerlijk gezegd not done om een van de members te gebruiken om een speciale state aan te geven... maak dan een nieuwe member aan

[Verwijderd]

(de volledige video_buffer struct ziet er zo uit:)

code:

struct video_buffer
{
    void    *base;
    int height,width;
    int depth;
    int bytesperline;
};

bytesperline = width * ((depth+7) / 8);

height/width/depth is dus de size/bitdepth van de buffer.

[drm]

<sub><miereneuk>[quote]
.oisyn:
Mits je niet al teveel genoeg geheugenruimte tot je beschikking hebt, maar dat betwijfel ik
[/quote]

mits beschrijft een positieve voorwaarde (op voorwaarde dat ...)
</miereneuk></sub>

beelzebubu>
Mijn vermoeden is, dat de struct een byte bevat om te kijken of de buffer geset is met interessante informatie.

Als je geheugen reserveert voor de pointer *base in struct video_buffer, en dat mislukt, komt base op NULL te staan. Ook zal hij wel geinitialiseerd worden op NULL.

Als je dan aan die informatie dezelfde betekenis als buffer_set hecht, weet je dus nooit wanneer de buffer ook daadwerkelijk informatie bevat, of alleen nog maar een toegewezen stuk geheugen is. Snap je?

[marcusk]

<sub>[miereneukmode][quote]
Mits je niet al teveel genoeg geheugenruimte tot je beschikking hebt, maar dat betwijfel ik
[/quote]

nee, zo klopt ie nog niet het moet zijn[quote]
Tenzij je niet al teveel geheugenruimte tot je beschikking hebt, maar dat betwijfel ik
[/quote]


[/miereneukmode]</sub>
verder heb ik niets nuttigs toe te voegen

[drm]

<sub>[miereneukmode][quote]
marcusk:
nee, zo klopt ie nog niet het moet zijn


verder heb ik niets nuttigs toe te voegen
[/quote]

lol, ja je hebt gelijk...
of
'mits je genoeg geheugenruimte hebt, maar daar twijfel ik niet aan'
[/miereneukmode]</sub>

[.oisyn]

Op vrijdag 31 mei 2002 13:53 schreef drm het volgende:
<sub>[miereneukmode]
[..]

lol, ja je hebt gelijk...
of
'mits je genoeg geheugenruimte hebt, maar daar twijfel ik niet aan'
[/miereneukmode]</sub>

nee want nu draai je de stelling om. Je moet het alleen doen als je niet genoeg geheugenruimte hebt
hmm volgens mij is mijn eerste stelling ook gewoon goed hoor

[drm]

.oisyn:
nee want nu draai je de stelling om. Je moet het alleen doen als je niet genoeg geheugenruimte hebt

Nee, je begrijpt het verkeerd.
Mits betekent:
op voorwaarde dat...

Oftewel:
op voorwaarde dat je genoeg geheugenruimte hebt, maar daar twijfel ik niet aan

geloof me nou maar

[JayTaph]

>Mijn vermoeden is, dat de struct een byte bevat om te
>kijken of de buffer geset is met interessante informatie.
>Als je geheugen reserveert voor de pointer *base in struct
>video_buffer, en dat mislukt, komt base op NULL te staan.
>Ook zal hij wel geinitialiseerd worden op NULL.
>Als je dan aan die informatie dezelfde betekenis als
>buffer_set hecht, weet je dus nooit wanneer de buffer ook
>daadwerkelijk informatie bevat, of alleen nog maar een
>toegewezen stuk geheugen is. Snap je?

Waarschijnlijk, (maar goed, ik ken de driver niet), moet je die is_set een beetje zien als soort semafoor. Op het moment dat iets of iemand de structures vult, kan het voorkomen dat er ondertussen een nieuw process actief wordt die deze gegevens al wil benaderen. Door alleen te kijken of de buffer wel of niet NULL is, is dus niet genoeg aangezien er nog andere variabelen kunnen zijn die nog niet gevuld zijn.

Op het moment dat alles gevuld is, zet je je "is_set" op 1, en vanaf dat punt weet je zeker dat de complete structure legale waardes bevatten.

[.oisyn]

ja ik weet wat het betekent
maar ik zie nu idd dat mijn zinsconstructie niet klopte. De mits sloeg op het feit dat je juist geen extra variabele gebruikte, maar uit mijn zinsconstructie blijkt het tegenovergestelde

[Soultaker]

Op vrijdag 31 mei 2002 14:49 schreef JayTaph het volgende:
Op het moment dat alles gevuld is, zet je je "is_set" op 1, en vanaf dat punt weet je zeker dat de complete structure legale waardes bevatten.

Datzelfde kun je natuurlijk wel afspreken over de buffer (die je dan dus pas als laatste invult).

Zelf ben ik het in principe eens met .oisyn's argumentatie, maar als je de struct een beetje klein wilt houden ('k weet niet in hoeverre dat een issue is), zou ik overwegen in plaats van aparte boolean variabelen een bitfield aan te maken met flags voor de verschillende onderdelen die wel of niet beschikbaar zijn.

Dit is trouwens ook beter portable, hoewel je dat probleem ook kunt afvangen door de grootte/versie van je datastructuur op te nemen (als eerste element of iets dergelijks).

[JayTaph]

>Datzelfde kun je natuurlijk wel afspreken over de buffer
>(die je dan dus pas als laatste invult).

Ja, maar een aparte var is duidelijker

[drm]

.oisyn:
ja ik weet wat het betekent
maar ik zie nu idd dat mijn zinsconstructie niet klopte. De mits sloeg op het feit dat je juist geen extra variabele gebruikte, maar uit mijn zinsconstructie blijkt het tegenovergestelde

hehe, we zijn er uit

[Verwijderd]

Op vrijdag 31 mei 2002 09:32 schreef drm het volgende:
Mijn vermoeden is, dat de struct een byte bevat om te kijken of de buffer geset is met interessante informatie.

Nou, nee dus... .

Als je dan aan die informatie dezelfde betekenis als buffer_set hecht, weet je dus nooit wanneer de buffer ook daadwerkelijk informatie bevat, of alleen nog maar een toegewezen stuk geheugen is. Snap je?

Nou, de buffer hoef je niet te allocaten. 'base' is een pointer naar de framebuffer, die hoeft een TV kaart driver niet te allocaten. Als die pointer dus geset is, dan is het een pointer naar een framebuffer. Anders is het een pointer naar niks (NULL dus).

[drm]

ah ok... op zulk een fiets... ok, dan maakt het idd niet zo veel uit.