Domanda

Ho un protocollo che richiede un campo di lunghezza fino a 32 bit e deve essere generato in fase di esecuzione per descrivere il numero di byte presenti in un determinato pacchetto.

Il codice qui sotto è un po' brutto, ma mi chiedo se questo può essere rifattorizzato essere leggermente più efficiente o facilmente comprensibile.Il problema è che il genererà solo un numero sufficiente di byte per descrivere la lunghezza del pacchetto, quindi minore di 255 byte = 1 byte di lunghezza, minore di 65535 = 2 byte di lunghezza and so on...

{
    extern char byte_stream[];
    int bytes = offset_in_packet;
    int n = length_of_packet;
   /* Under 4 billion, so this can be represented in 32 bits. */
    int t;
   /* 32-bit number used for temporary storage. */

    /* These are the bytes we will break up n into. */
    unsigned char first, second, third, fourth;

    t = n & 0xFF000000;
    /* We have used AND to "mask out" the first byte of the number. */
    /* The only bits which can be on in t are the first 8 bits. */
    first = t >> 24;
    if (t)  {
        printf("byte 1: 0x%02x\n",first );
        byte_stream[bytes] = first; bytes++;
        write_zeros = 1;
    }
    /* Now we shift t so that it is between 0 and 255. This is the first, highest byte of n. */
    t = n & 0x00FF0000;
    second = t >> 16;
    if (t || write_zeros) {
        printf("byte 2: 0x%02x\n", second );
        byte_stream[bytes] = second; bytes++;
        write_zeros = 1;
    }

    t = n & 0x0000FF00;
    third = t >> 8;
    if ( t || write_zeros) {
        printf("byte 3: 0x%02x\n", third );
        byte_stream[bytes] = third; bytes++;
        write_zeros = 1;
    }

    t = n & 0x000000FF;
    fourth = t;
    if (t || write_zeros) {
        printf("byte 4: 0x%02x\n", fourth);
        byte_stream[bytes] = fourth; bytes++;
    }
}
È stato utile?

Soluzione

In realtà stai facendo solo quattro calcoli, quindi la leggibilità sembra molto più importante qui che efficienza.Il mio approccio per rendere qualcosa di simile più leggibile è quello di

  1. Estrai il codice comune in una funzione
  2. Metti insieme calcoli simili per rendere i modelli più evidenti
  3. Sbarazzati della variabile intermedia print_zeroes e sii esplicito riguardo ai casi in cui restituisci byte anche se sono zero (ad es.il byte precedente era diverso da zero)

Ho trasformato il blocco di codice casuale in una funzione e modificato alcune variabili (i caratteri di sottolineatura mi danno problemi nella schermata di anteprima del markdown).Lo avevo supposto anch'io byte viene trasmesso e che chiunque lo stia trasmettendo ci passerà un puntatore in modo che possiamo modificarlo.

Ecco il codice:

/* append byte b to stream, increment index */
/* really needs to check length of stream before appending */
void output( int i, unsigned char b, char stream[], int *index )
{
    printf("byte %d: 0x%02x\n", i, b);
    stream[(*index)++] = b;
}


void answer( char bytestream[], unsigned int *bytes, unsigned int n)
{
    /* mask out four bytes from word n */
    first  = (n & 0xFF000000) >> 24;
    second = (n & 0x00FF0000) >> 16;
    third  = (n & 0x0000FF00) >>  8;
    fourth = (n & 0x000000FF) >>  0;

    /* conditionally output each byte starting with the */
    /* first non-zero byte */
    if (first) 
       output( 1, first, bytestream, bytes);

    if (first || second) 
       output( 2, second, bytestream, bytes);

    if (first || second || third) 
       output( 3, third, bytestream, bytes);

    if (first || second || third || fourth) 
       output( 4, fourth, bytestream, bytes);
 }

Mai così leggermente più efficiente, e Forse più facile da capire sarebbe questa modifica alle ultime quattro istruzioni if:

    if (n>0x00FFFFFF) 
       output( 1, first, bytestream, bytes);

    if (n>0x0000FFFF) 
       output( 2, second, bytestream, bytes);

    if (n>0x000000FF)  
       output( 3, third, bytestream, bytes);

    if (1) 
       output( 4, fourth, bytestream, bytes);

Sono d'accordo, tuttavia, che la compressione di questo campo rende la macchina dello stato ricevente eccessivamente complicata.Ma se non puoi modificare il protocollo, questo codice è molto più facile da leggere.

Altri suggerimenti

Dovresti davvero utilizzare un campo a larghezza fissa per la tua lunghezza.

  • Quando il programma sul lato ricevente deve leggere il campo della lunghezza del pacchetto, come fa a sapere dove si ferma la lunghezza?
  • Se la lunghezza di un pacchetto può potenzialmente raggiungere i 4 GB, è davvero importante un sovraccarico di 1-3 byte?
  • Vedi quanto è già diventato complesso il tuo codice?

Prova questo ciclo:

{
    extern char byte_stream[];
    int bytes = offset_in_packet;
    int n = length_of_packet; /* Under 4 billion, so this can be represented in 32 bits. */
    int t; /* 32-bit number used for temporary storage. */
    int i;

    unsigned char curByte;

    for (i = 0; i < 4; i++) {
        t = n & (0xFF000000 >> (i * 16));

        curByte = t >> (24 - (i * 8));
        if (t || write_zeros)  {
            printf("byte %d: 0x%02x\n", i, curByte );
            byte_stream[bytes] = curByte;
                            bytes++;
            write_zeros = 1;
        }

    }

}

Non sono sicuro di aver capito la tua domanda.Cosa stai cercando di contare esattamente?Se ho capito bene, stai cercando di trovare il byte diverso da zero più significativo.
Probabilmente faresti meglio a usare un ciclo come questo:

int i;  
int write_zeros = 0;  
for (i = 3; i >=0 ; --i) {  
    t = (n >> (8 * i)) & 0xff;  
    if (t || write_zeros) {  
        write_zeros = 1;  
        printf ("byte %d : 0x%02x\n", 4-i, t);  
        byte_stream[bytes++] = t;
    }  
}
Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top