Comments 25
UFO landed and left these words here
Во-первых огребёте с выравниванием, т.к. никто не сказал что пришедший указатель data — выравнен по 2 байта

Оригинальный код на эту тему тоже не парится, так что норм.
Во-вторых, нужно сделать код endianess-нейтральным, а не так как сейчас

Я бы сделал так:
uint16_t Echo::icmpChecksum(const char *data, int length)
{
    uint16_t *data16 = (uint16_t *)data;
    uint32_t sum = 0;
    
    for (sum = 0; length > 1; length -= 2)
        sum += *data16++;
    if (length == 1) {
        char tail[2] = {*(char *)data16, 0};
        sum += *(uint16_t *)tail;
    }
    sum = (sum >> 16) + (sum & 0xffff);
    sum += (sum >> 16);
    return ~sum;
}
UFO landed and left these words here
Традиционно один патч — одна проблема. Этот патч фиксит неправильно считаемую контрольную сумму.
Вы предлагаете патч для оптимизации. Обычно вместе с этим требуется какое-нибудь обоснование: цифры, показывающие улучшение.
UFO landed and left these words here
проблема одна — ф-ция не нейтральна к endianess + возможен доступ к невыравненным

Плюс между слагаемыми просто оттеняет то, что проблема одна…
UFO landed and left these words here
Т.е. если автор hanstunnel поговнокодил — можно говнокодить? Я хотя бы assert-ов добавил бы, на то что указатель выравненный.

Тогда уж надо вообще всю прогу рефакторить. Только вот с таким подходом все время уйдет на один сплошной рефакторинг.
Оригинал не смотрел

Не читал, но осуждаю (с)

Смотрите код целиком. Не будет gcc выделять по new блок с плохим выравниванием. Код и так endianess-нейтральный. Сетевой контрольной сумме пофигу на endian, а последний байт ставится на место через ntohs.
UFO landed and left these words here
Изменения на портабельность не повлияли. Допущения обеспечивает класс, которому принадлежит метод. Кстати, если код не портабелен, то как он работает на Intel, ARM и MIPS?
UFO landed and left these words here
Я просто пытаюсь донести, что этот код в том контексте, в котором он используется в приложении, не нуждается в дополнительных assert'ах и проверках. Без контекста и к i++ придраться можно, ибо, например, не thread-safe.
Кстати, если напишете тесты, которые покрашат hanstunnel на MIPS и ARM, будет интересно.
UFO landed and left these words here
Кстати, можно тогда посмотреть на пример вашего решения данной проблемы? То есть как бы на моем месте проблему решали вы? Имеется в виду в данном конкретном случае.
UFO landed and left these words here
Основное отличие тут — побайтовое чтение. Сам по себе порядок байтов абсолютно не важен в силу алгоритма подсчета.

Теперь смотрим единственное использование метода.

    EchoHeader *header = (EchoHeader *)(sendBuffer + sizeof(IpHeader));
    header->type = reply ? 0: 8;
    header->code = 0;
    header->id = htons(id);
    header->seq = htons(seq);
    header->chksum = 0;
    header->chksum = icmpChecksum(sendBuffer + sizeof(IpHeader), payloadLength + sizeof(EchoHeader));


где IpHeader имеет размер 20 байт, а sendBuffer выделен по new в конструкторе. То есть смещение всегда четное.

Предлагаю сойтись на том, что проверки и побайтовое чтение нужны в общем случае, особенно, если код отдается как библиотека.
UFO landed and left these words here
Теоретическая часть по ссылке из поста отлично объясняет, почему суммирование с циклическим переносом работает одинаково и на be и на le. Рекомендую ознакомиться.

TL;DR: смена порядка байт при чтении компенсируется сменой порядка байт при записи, а перенос всё равно циклический.
UFO landed and left these words here
А можно готовый вариант в виде устанавливаемого пакета для чайников?
Only those users with full accounts are able to leave comments. Log in, please.