Descripción de la longitud del nivel de bytes

Question

Tengo un protocolo que requiere un campo de longitud de hasta 32 bits, y debe generarse en tiempo de ejecución para describir cuántos bytes hay en un paquete determinado.

El código a continuación es un poco feo, pero me pregunto si esto puede refactorizarse para que sea un poco más eficiente o fácilmente comprensible.El problema es que el código solo generará suficientes bytes para describir la longitud del paquete, por lo que menos de 255 bytes = 1 byte de longitud, menos de 65535 = 2 bytes de longitud, etc.

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

Answer 1

Realmente sólo estás haciendo cuatro cálculos, así que la legibilidad parece mucho más importante aquí que la eficiencia.Mi enfoque para hacer que algo como esto sea más legible es

1. Extraer código común a una función.
2. Junte cálculos similares para hacer que los patrones sean más obvios.
3. Deshágase de la variable intermedia print_zeroes y sea explícito acerca de los casos en los que genera bytes incluso si son cero (es decir,el byte anterior era distinto de cero)

Cambié el bloque de código aleatorio a una función y cambié algunas variables (los guiones bajos me están dando problemas en la pantalla de vista previa de rebajas).También he asumido que bytes se está pasando, y que quien lo esté pasando nos pasará un puntero para que podamos modificarlo.

Aquí está el código:

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

Un poco más eficiente y tal vez Más fácil de entender sería esta modificación de las últimas cuatro declaraciones 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);

Sin embargo, estoy de acuerdo en que comprimir este campo hace que la máquina de estado receptor sea demasiado complicada.Pero si no puedes cambiar el protocolo, este código es mucho más fácil de leer.

Answer 2

Realmente deberías usar un campo de ancho fijo para tu longitud.

• Cuando el programa en el extremo receptor tiene que leer el campo de longitud de su paquete, ¿cómo sabe dónde termina la longitud?
• Si la longitud de un paquete puede alcanzar potencialmente los 4 GB, ¿realmente importa una sobrecarga de 1 a 3 bytes?
• ¿Ves lo complejo que ya se ha vuelto tu código?

Answer 3

Prueba este bucle:

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

    }

}

Answer 4

No estoy seguro de entender tu pregunta.¿Qué estás tratando de contar exactamente?Si entiendo correctamente, estás intentando encontrar el byte distinto de cero más significativo.
Probablemente sea mejor que uses un bucle como este:

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