Frage

Ich habe ein Protokoll, das ein Längenfeld bis zu 32 Bit erfordert, und es muss zur Laufzeit generiert werden, um zu beschreiben, wie viele Bytes in einem bestimmten Paket enthalten sind.

Der folgende Code ist etwas hässlich, aber ich frage mich, ob dies etwas effizienter oder leicht verständlicher ist.Das Problem ist, dass der Code nur genügend Bytes generiert, um die Länge des Pakets zu beschreiben, also weniger als 255 Bytes = 1 Byte der Länge, weniger als 65535 = 2 Bytes Länge usw.

{
    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++;
    }
}
War es hilfreich?

Lösung

Eigentlich führen Sie also nur vier Berechnungen durch Lesbarkeit scheint viel wichtiger zu sein hier als Effizienz.Mein Ansatz, um so etwas lesbarer zu machen, ist:

  1. Extrahieren Sie allgemeinen Code in eine Funktion
  2. Fassen Sie ähnliche Berechnungen zusammen, um die Muster deutlicher zu machen
  3. Entfernen Sie die Zwischenvariable print_zeroes und geben Sie explizit an, in welchen Fällen Sie Bytes ausgeben, auch wenn diese Null sind (d. h.das vorhergehende Byte war ungleich Null)

Ich habe den Zufallscodeblock in eine Funktion umgewandelt und einige Variablen geändert (Unterstriche bereiten mir Probleme im Markdown-Vorschaubildschirm).Das habe ich auch angenommen Bytes übergeben wird, und dass derjenige, der es übergibt, uns einen Zeiger übergibt, damit wir ihn ändern können.

Hier ist der Code:

/* 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);
 }

Etwas effizienter und Vielleicht leichter zu verstehen wäre diese Modifikation der letzten vier if-Anweisungen:

    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);

Ich stimme jedoch zu, dass die Komprimierung dieses Felds die empfangende Zustandsmaschine übermäßig kompliziert macht.Wenn Sie das Protokoll jedoch nicht ändern können, ist dieser Code viel einfacher zu lesen.

Andere Tipps

Sie sollten wirklich ein Feld mit fester Breite für Ihre Länge verwenden.

  • Wenn das Programm auf der Empfängerseite das Längenfeld Ihres Pakets lesen muss, woher weiß es dann, wo die Länge aufhört?
  • Wenn die Länge eines Pakets möglicherweise 4 GB erreichen kann, spielt ein Overhead von 1 bis 3 Byte wirklich eine Rolle?
  • Sehen Sie, wie komplex Ihr Code bereits geworden ist?

Probieren Sie diese Schleife aus:

{
    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;
        }

    }

}

Ich bin mir nicht sicher, ob ich Ihre Frage verstehe.Was genau versuchst du zu zählen?Wenn ich das richtig verstehe, versuchen Sie, das höchstwertige Byte ungleich Null zu finden.
Sie sind wahrscheinlich besser dran, eine Schleife wie diese zu verwenden:

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;
    }  
}
Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top