Pointer in C

[Oliver L. (ollil)]

Hi,

in meinem Programm muss ich Daten von einem Port abholen und dabei 
diverse andere Pin-Stati beachten.
Es ist so, das die Anzahl der Daten durch ein vorheriges Lesen des Ports 
ermittelt werden.

1. Lese Port, ermittle Anzahl abzuholende Bytes
Ergebniss ist immer ein vielfaches von 2!
2. Lese Port erneut und zwar so oft wie Bytes abzuholen sind.

Ich habe mir am Anfang einfach ein großes Array deklariert mit der 
maximalen abzuholenden Größe die auftreten wird.

Ich wollte nun dazu übergehen, den Code zu modularisieren und auch mit 
Pointern und dynamischer Allokierung der Daten arbeiten. Folgender 
relevanter Codeabschnitt:

int main(void);
    uint8_t *cmdbuffer = NULL;

    wdc_read_data_from_p8k(&cmdbuffer
                          ,9
                          ,INFO_STAT_GCMD
    );
}

void wdc_read_data_from_p8k(uint8_t **buffer, uint8_t count, uint8_t wdc_status)
{
    int datacnt;
     
    (*buffer) = (uint8_t *)malloc(count * sizeof(**buffer));
    configure_port_data_read();
    datacnt = 0;
    port_info_set( wdc_status );
    while ( !isset_info_te() );
    while ( !isset_info_wdardy() );
    while ( datacnt < count )
    {
        while ( isset_info_wdardy() );
        port_info_set( INFO_ASTB | wdc_status );
        (*buffer)[datacnt] = PINA;
        port_info_set( wdc_status );
        while ( !isset_info_wdardy() );
        datacnt++;
    }
    port_info_set( INFO_CLEAR );
}

Kann mir erstmal einer sagen, ob das ganze korrektem C-Code entspricht?

Wenn ich buffer global als "uint8_t buffer[9]" deklariere, und auf 
malloc verzichte und direkt verwende in der Funktion, werden folgende 
Bytes gelesen:

18002eaaaa550200ff

Das ist korrekt.
Mit meiner Pointer-Lösung scheinen Bytes verschluckt zu werden:

18002eaaaa02ff1800

Ich denke, das kann zwei Ursachen haben:
1.) Die Zuweisung "(*buffer)[datacnt] = PINA" braucht mehr Zeit als 
"buffer[datacnt] = PINA" und da gehen dadurch Bytes verloren
2.) Das Array wird nicht korrekt allokiert. Bzw. mein C-Code hat einfach 
semantische Fehler.

Bin für jeden Tipp offen.

[Karl H. (kbuchegg)]

Oliver Lehmann schrieb:

> Kann mir erstmal einer sagen, ob das ganze korrektem C-Code entspricht?

Jetztr nur auf das Array bzw. die Pointer Verwendung bezogen:
Ja, sieht gut aus

Den Rest hab ich mir nicht anbgesehen.

> Wenn ich buffer global als "uint8_t buffer[9]" deklariere, und auf
> malloc verzichte und direkt verwende in der Funktion, werden folgende
> Bytes gelesen:
>
> 18002eaaaa550200ff
>
> Das ist korrekt.
> Mit meiner Pointer-Lösung scheinen Bytes verschluckt zu werden:
>
> 18002eaaaa02ff1800
>
> Ich denke, das kann zwei Ursachen haben:
> 1.) Die Zuweisung "(*buffer)[datacnt] = PINA" braucht mehr Zeit als
> "buffer[datacnt] = PINA" und da gehen dadurch Bytes verloren

Das kann sein.
Wenn dein Code so zeitkritisch ist, müsste man sich da was anderes 
einfallen lassen.
Welche zeitlichen Nebenbedingungen hast du denn?

> 2.) Das Array wird nicht korrekt allokiert. Bzw. mein C-Code hat einfach
> semantische Fehler.

Nö, sieht gut aus.

>
> Bin für jeden Tipp offen.

[Klaus W. (mfgkw)]

Das sieht mir auch korrekt aus.

Wenn es wirklich einen Unterschied gibt, würde ich darauf tippen, daß 
die erste Version falsch ist.
Wenn du es so geändert hast, wie ich vermute (mit dem globalen Puffer), 
wäre es eh falsch - aber den Code sehe ich ja nicht.

[Andreas B. (andreasb)]

Ich habe den Beitrag nur überflogen, und ein "Fehler" entdeckt, der zwar 
dein Problem nicht lösen, aber ggf. trotzdem etwas bringen...

>    int datacnt;

wäre doch besser ein uint8_t?

Ich gehe mal von einem AVR aus, da ist "int" 16bit, und somit langsamer 
als uint8_t.

Zu 1)
ggf. könntest du uint8_t * buffer2 = *buffer;

schreibe, und dann buffer2 verwenden, ich bin mir aber nicht sicher ob 
das schneller ist...


mfg Andreas

[Peter II (Gast)]

das sollt schneller sein

 uint8_t* akt_pos = buffer;
 while ( datacnt < count )
    {
        while ( isset_info_wdardy() );
        port_info_set( INFO_ASTB | wdc_status );
        *akt_pos  = PINA;
        akt_pos++;
        port_info_set( wdc_status );
        while ( !isset_info_wdardy() );
        datacnt++;
    }

[Andreas B. (andreasb)]

Auf Intel Rechnernen villeicht.

Wenn es aber auf einem AVR läuft ist akt_pos++; eine 16 bit 
Inkrementation.

Wird hingegen mit einem 8bit Offset auf ein Array zugegriffen ist dies 
schneller, und der Compiler legt dies meistens auch noch in ein 
Register, kann ansonsten mit "register" gefördert (aber nicht erzwungen) 
werden.

Darum wenn möglich 8bit Offset und keine Pointer Arithmetik, den Pointer 
sind 16bittig auf AVRs...


mfg Andreas

Peter II schrieb:
> das sollt schneller sein
>
>  uint8_t* akt_pos = buffer;
>  while ( datacnt < count )
>     {
>         while ( isset_info_wdardy() );
>         port_info_set( INFO_ASTB | wdc_status );
>         *akt_pos  = PINA;
>         akt_pos++;
>         port_info_set( wdc_status );
>         while ( !isset_info_wdardy() );
>         datacnt++;
>     }

[Oliver L. (ollil)]

So...

ich habe nun folgenden Code, dieser wird korrekt ausgeführt:

Die 9 ist natürlich nur ein Beispiel - es können bis zu 4096 Bytes 
übertragen werden.

int main(void)
{
    uint16_t cmd_counter = 9;
    uint8_t *cmd_buffer = NULL;

    cmd_buffer = malloc(cmd_counter*sizeof(cmd_buffer));

    wdc_read_data_from_p8k(cmd_buffer
                          ,cmd_counter
                          ,INFO_STAT_GCMD
                          );
}

void wdc_read_data_from_p8k(uint8_t *buffer, uint8_t count, uint8_t wdc_status)
{
    int datacnt;
   
    configure_port_data_read();
    datacnt = 0;
    port_info_set( wdc_status );
    while ( !isset_info_te() );
    while ( !isset_info_wdardy() );
    while ( datacnt < count )
    {
        while ( isset_info_wdardy() );
        port_info_set( INFO_ASTB | wdc_status );
        buffer[datacnt] = PINA;
        port_info_set( wdc_status );
        while ( !isset_info_wdardy() );
        datacnt++;
    }
    port_info_set( INFO_CLEAR );
    
}

Ich hätte halt nur  den malloc() gern mit in der Funktion gekappselt....
Achja - es ist ein ATMega1284P

[Peter II (Gast)]

Andreas B. schrieb:
> Auf Intel Rechnernen villeicht.
> Wenn es aber auf einem AVR läuft ist akt_pos++; eine 16 bit
> Inkrementation.

aber ein pointer im AVR ist auch > 8bit. Und damit muss auch eine 
Operation mit 8 bit mit überlauf gemacht werden.

Bei der original version muss er sich 2 zeiger merken, einmal den anfang 
für den buffer und einmal die Position wo er hinscreiben muss, damit 
braucht er 2 pointer register (und ich glaube er nutzt nur eines). Damit 
kann er es nicht in den registern halten und muss ständig den zeiger auf 
den anfang neu laden.

Eventuell kann ja jemand mal den ASM code von beiden Varianten posten, 
habe gerade keine passene umgebung da.

[Karl H. (kbuchegg)]

Oliver Lehmann schrieb:

> Ich hätte halt nur  den malloc() gern mit in der Funktion gekappselt....

Was in C meistens gar keine so gute Idee ist.

[Andreas B. (andreasb)]

Oliver Lehmann schrieb:
> Die 9 ist natürlich nur ein Beispiel - es können bis zu 4096 Bytes
> übertragen werden.

Dann reicht aber der uint8_t nicht mehr!

Peter II schrieb:
> Andreas B. schrieb:
>> Auf Intel Rechnernen villeicht.
>> Wenn es aber auf einem AVR läuft ist akt_pos++; eine 16 bit
>> Inkrementation.
>
> aber ein pointer im AVR ist auch > 8bit.

Ja, 16 bit, das meinte ich oben...

> Und damit muss auch eine
> Operation mit 8 bit mit überlauf gemacht werden.

Genau, der AVR kann aber mit einem 8bit Offset zugreifen, dann musst du 
auch nur diesen Offset inkrementieren. (der "Basepointer" kann in einem 
Register bleiben, er ändert ja nicht)

Wurde hier im Forum mal komplett analysiert, weiss nicht mehr genau 
welcher Thread.

Es hat sich aber aber sowieso erledigt, denn bei 4096 braucht er sowieso 
einen 16bit Counter.

Wahrscheinlich dürfte dann die Pointerversion schneller sein, da er es 
direkt mit einem Pointer machen kann...


mfg Andreas

[free() (Gast)]

Karl Heinz Buchegger schrieb:
> Was in C meistens gar keine so gute Idee ist.

... weil z.B. noch die Frage im Raum steht, wer den allozierten Speicher 
wieder freigibt.

[Oliver L. (ollil)]

Andreas B. schrieb:
> Oliver Lehmann schrieb:
>> Die 9 ist natürlich nur ein Beispiel - es können bis zu 4096 Bytes
>> übertragen werden.
>
> Dann reicht aber der uint8_t nicht mehr!

Du meinst bei der Deklaration der Function weil dort count als uint8_t 
deklariert ist (was in der Tat falsch ist), oder bei der Deklaration von 
cmd_buffer?



Hier mal die ASM-Quellen:

mit malloc in der Funktion (1. Posting)
http://files.pofo.de/main_1.s

mit malloc vor der Funktion
http://files.pofo.de/main_2.s

[Karl H. (kbuchegg)]

free() schrieb:
> Karl Heinz Buchegger schrieb:
>> Was in C meistens gar keine so gute Idee ist.
>
> ... weil z.B. noch die Frage im Raum steht, wer den allozierten Speicher
> wieder freigibt.

Genau. Ist zb ein Thema.

Ein anderes Thema lautet: Was soll die Funktion machen, wenn sie den 
Speicher nicht bekommt?
Wieder ein anderes Thema lautet: Wenn die Funktion den Speicher einfach 
vorgegeben bekommt, dann kann ich ihr dynamisch allokierten Speicher 
aber auch ein fix allokiertes Array als Speicherfläche anbieten. D.h. 
aus Softwaresicht ist diese Variante universeller benutzbar.

[Walter S. (avatar)]

Oliver Lehmann schrieb:
> sizeof(cmd_buffer)

das ist nicht das was du willst, das gibt dir die Größe eines Pointers,
dein array besteht aber nicht aus pointern!

[Oliver L. (ollil)]

Walter S. schrieb:
> Oliver Lehmann schrieb:
>> sizeof(cmd_buffer)
>
> das ist nicht das was du willst, das gibt dir die Größe eines Pointers,
> dein array besteht aber nicht aus pointern!

besser sizeof(uint8_t)?

[Karl H. (kbuchegg)]

Oliver Lehmann schrieb:
> Walter S. schrieb:
>> Oliver Lehmann schrieb:
>>> sizeof(cmd_buffer)
>>
>> das ist nicht das was du willst, das gibt dir die Größe eines Pointers,
>> dein array besteht aber nicht aus pointern!
>
> besser sizeof(uint8_t)?


sizeof(*cmd_buffer)