문제

최대 32 비트의 길이 필드가 필요한 프로토콜이 있습니다. 주어진 패킷에 얼마나 많은 바이트가 있는지 설명하기 위해 런타임에 생성됩니다.

아래 코드는 다소 추악하지만 리팩토링할 수 있는지 궁금합니다. 약간 더 효율적이거나 쉽게 이해할 수 있어야 합니다.문제는 코드는 패킷의 길이를 설명하기에 충분한 바이트만 생성하므로 255바이트 미만 = 1바이트 길이, 65535 미만 = 2바이트 길이 등...

{
    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++;
    }
}
도움이 되었습니까?

해결책

실제로는 네 가지 계산만 수행하고 있으므로 가독성이 훨씬 더 중요해 보입니다 여기서는 효율성보다이와 같은 내용을 더 읽기 쉽게 만드는 나의 접근 방식은 다음과 같습니다.

  1. 공통 코드를 함수로 추출
  2. 유사한 계산을 함께 사용하여 패턴을 더욱 명확하게 만듭니다.
  3. 중간 변수 print_zeroes를 제거하고 바이트가 0이더라도 바이트를 출력하는 경우를 명시적으로 설명하세요(예:이전 바이트가 0이 아님)

무작위 코드 블록을 함수로 변경하고 몇 가지 변수를 변경했습니다(마크다운 미리보기 화면에서 밑줄로 인해 문제가 발생했습니다).나도 그럴 거라고 추측했어 바이트 전달되고 있으며, 전달하는 사람은 누구나 수정할 수 있도록 포인터를 전달합니다.

코드는 다음과 같습니다.

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

조금 더 효율적이고 아마도 이해하기 더 쉬운 것은 마지막 4개의 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);

그러나 이 필드를 압축하면 수신 상태 머신이 지나치게 복잡해진다는 점에는 동의합니다.그러나 프로토콜을 변경할 수 없는 경우 이 코드를 읽는 것이 훨씬 쉽습니다.

다른 팁

실제로 길이에 고정 너비 필드를 사용해야 합니다.

  • 수신측 프로그램이 패킷의 길이 필드를 읽어야 할 때 길이가 어디에서 멈추는지 어떻게 알 수 있습니까?
  • 패킷 길이가 잠재적으로 4GB에 도달할 수 있다면 1-3바이트 오버헤드가 실제로 문제가 됩니까?
  • 코드가 이미 얼마나 복잡해졌는지 보셨나요?

다음 루프를 시도해 보세요.

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

    }

}

귀하의 질문을 이해했는지 잘 모르겠습니다.정확히 무엇을 계산하려고 합니까?내가 올바르게 이해했다면 0이 아닌 가장 중요한 바이트를 찾으려고 하는 것입니다.
다음과 같은 루프를 사용하는 것이 더 나을 것입니다.

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;
    }  
}
라이센스 : CC-BY-SA ~와 함께 속성
제휴하지 않습니다 StackOverflow
scroll top