[C]Dove sto sbagliando?

[stefanoxjx]

Ciao a tutti, dopo qualche anno di inattività, sto sviluppando un programma in C, però mi sono arenato in un punto e non riesco a capire dove sto sbagliando

Praticamente, devo leggere da un dispositivo seriale dei dati, ogni pacchetto inizia con 0x7e e finisce con 0x7e ed è lungo 11 byte HEADER compresi.
Il secondo byte del pacchetto (0xfe, 0xfd) definisce il tipo di dato ricevuto.
Questo è il programma:

void	ParseAnalog(void);
void	ParseHUB(void);

#define HEADER	'\x7e'
#define ANALOG	'\xfe'
#define	HUB	    '\xfd'
//#define	HUB	'\x5e'
#define PACKETSIZE  11

int  port;
unsigned	int 	packet[PACKETSIZE+1];

int main(void)
{
    int 	i=0;
    unsigned char buf[2];

    port=ComOpen();

    while(1)
    {
        buf[0]=0;
        if(read(port,buf, 1) > 0)
        {
//            printf("%d\n",i);
 
            if(i==10 && buf[0]==HEADER && packet[0]==HEADER)
            {

                if((int)packet[1]==(int)ANALOG)
                    printf("ParseAnalog\n");
                    //ParseAnalog();
                //printf("i=10\n");
                //if(packet[1]==ANALOG)
                    //ParseAnalog();
                //if(packet[1]==HUB)
                    //ParseHUB();
            }

            if(buf[0]==HEADER && i < PACKETSIZE) i=0;

            packet[i++]=buf[0];

            if(i >= PACKETSIZE) i=0;
        }
    }
}

void ParseAnalog(void)
{
    printf("Parte analogica\n");
}

void ParseHUB(void)
{
    printf ("**************** DATI HUB ***************\n");
}

Il problema sta nella parte in grassetto.
Praticamente, l'istruzione dopo if((int)packet[1]==(int)ANALOG) non viene mai eseguito nonostante packet[1] sia uguale ad ANALOG (0xfe).
Stessa cosa dicasi per le altre righe commentate che al momento sono così perchè sto cercando di capire il problema.
In cosa sto sbagliando?
Pensavo fosse un problema di casting, ma come potete vedere ho forzato le variabili ad INT e quindi anche il casting non può essere.

Grazie.
Ciao.

[pabloski]

Perchè (int)packet[1]?? Si tratta di 8 bit, quindi il cast va fatto con char.

[stefanoxjx]

Originariamente inviato da pabloski

Perchè (int)packet[1]?? Si tratta di 8 bit, quindi il cast va fatto con char.

Forse ricordo male, ma "(int)packet[1]" non dovrebbe convertire char in int?
Sono circa 10 anni che non programmo, quindi un po' qualcosa l'ho perso
Tra le altre cose, nel frattempo ho scoperto il mio problema.
Era "#define ANALOG '\xfe'" che una volta printato ho visto che ritornava -2.
Probabilmente sempre per lo stesso problema del quale a questo punto ho probabilmente un po' di confusione.

[pabloski]

Originariamente inviato da stefanoxjx

Forse ricordo male, ma "(int)packet[1]" non dovrebbe convertire char in int?

Si.

Originariamente inviato da stefanoxjx

Era "#define ANALOG '\xfe'" che una volta printato ho visto che ritornava -2.
Probabilmente sempre per lo stesso problema del quale a questo punto ho probabilmente un po' di confusione.

Un problema di segni. Quindi (unsigned int) risolve.

[stefanoxjx]

Originariamente inviato da pabloski

Si.

Un problema di segni. Quindi (unsigned int) risolve.

Negativo, (unsigned int) restituisce 4294967294, mentre (unsigned char) risolve
Grazie dell'aiuto

[WarDuck]

La cosa migliore che puoi fare quando hai un buffer il cui contenuto è più o meno fissato è definirti una struttura che descrive il buffer (quantomeno la parte fissa), e poi fare un unico cast con quello:

struct my_buff
{
   int a;
   int b;
   char c;
   int len;
   char data[1]; // segnaposto per i dati
};

int main(void)
{
    // retreive buffer

    struct my_buff* packet = (struct my_buff*) buffer;

    // packet->a;
    // packet->b;
    // packet->c;

}

[stefanoxjx]

Originariamente inviato da WarDuck

La cosa migliore che puoi fare quando hai un buffer il cui contenuto è più o meno fissato è definirti una struttura che descrive il buffer (quantomeno la parte fissa), e poi fare un unico cast con quello:

struct my_buff
{
   int a;
   int b;
   char c;
   int len;
   char data[1]; // segnaposto per i dati
};

int main(void)
{
    // retreive buffer

    struct my_buff* packet = (struct my_buff*) buffer;

    // packet->a;
    // packet->b;
    // packet->c;

}

Grazie, questa è una buona dritta.
Devo però ripassarmi le struct perchè mi ero completamente dimenticato della loro esistenza

[enaud]

ad occhio, non ho letto le risposte, prova ad usare unsigned char invece di int.

[stefanoxjx]

Originariamente inviato da enaud

ad occhio, non ho letto le risposte, prova ad usare unsigned char invece di int.

Si si, hai azzeccato la risposta.
Se leggi quanto scritto prima vedrai che confermo che unsigned char risolve

[marco.r]

Originariamente inviato da WarDuck

La cosa migliore che puoi fare quando hai un buffer il cui contenuto è più o meno fissato è definirti una struttura che descrive il buffer (quantomeno la parte fissa), e poi fare un unico cast con quello:

struct my_buff
{
   int a;
   int b;
   char c;
   int len;
   char data[1]; // segnaposto per i dati
};

int main(void)
{
    // retreive buffer

    struct my_buff* packet = (struct my_buff*) buffer;

    // packet->a;
    // packet->b;
    // packet->c;

}

Occhio che cosi' il codice diventa non portabile e, soprattutto, non funziona se viene scritto in una architettura e letto in un'altra.
Questo perche' a priori non sai che allineamento viene usato e quindi la dimensione di my_buff.
Senza contare che manco sizeof(int) e' definita.
Serializzare e deserializzare direttamente delle strutture non e' una buona pratica, a meno che non si tratti di una cosa interna al programma (e.g. parti diverse di un sistema operativo che comunicano tra di loro).
In ogni caso anche un array di int e' sbagliato (per il discorso di sizeof(int) detto piu' sopra).
Meglio usare un array di unsigned char, nella struttura dati usare possibilmente delle dimensioni esplicite (int32_t ad esempio) ed infine verficare il byte ordering dei numeri letti, usando ntohl e compagnia (eventualmente "invertendo" l'ordine prima, se on the wire non sono in network order).

[WarDuck]

Originariamente inviato da marco.r

Occhio che cosi' il codice diventa non portabile e, soprattutto, non funziona se viene scritto in una architettura e letto in un'altra.
Questo perche' a priori non sai che allineamento viene usato e quindi la dimensione di my_buff.

L'allineamento può essere indicato con le direttive del compilatore, comunque se non erro ci dovrebbero essere una serie di regole comuni al linguaggio C o comunque legate alla specifica implementazione del compilatore.

Certo se lavori ad un progetto cross-platform bisogna valutare questi aspetti, ma IMHO risulta enormemente più comodo lavorare in quel modo, chiaramente con la dovuta accortezza.

Anche perché una volta preparata la struttura ad esempio di un protocollo (se è a campi fissi), hai automaticamente accesso a tutti i campi.

Originariamente inviato da marco.r

Senza contare che manco sizeof(int) e' definita.
Serializzare e deserializzare direttamente delle strutture non e' una buona pratica, a meno che non si tratti di una cosa interna al programma (e.g. parti diverse di un sistema operativo che comunicano tra di loro).
In ogni caso anche un array di int e' sbagliato (per il discorso di sizeof(int) detto piu' sopra).
Meglio usare un array di unsigned char, nella struttura dati usare possibilmente delle dimensioni esplicite (int32_t ad esempio) ed infine verficare il byte ordering dei numeri letti, usando ntohl e compagnia (eventualmente "invertendo" l'ordine prima, se on the wire non sono in network order).

Il mio era un esempio per fargli vedere la potenzialità della tecnica, è sicuramente consigliabile usare i tipi definiti in stdint.h.

Riguardo al byte ordering bisogna vedere se questo può essere un problema o meno, a seconda dei casi.