[Oisf-devel] Coding style improvement proposal

[Jason Ish]

On Tue, May 12, 2015 at 9:05 AM, Eric Leblond <eric at regit.org> wrote:
> Hello,
>
> As some of you know, I don't like unittests. And when I see things like:
>
> int SMTPProcessDataChunkTest02(void){
>     char mimemsg[] = {0x4D, 0x49, 0x4D, 0x45, 0x2D, 0x56, 0x65, 0x72,
>             0x73, 0x69, 0x6F, 0x6E, 0x3A, 0x20, 0x31, 0x2E,
>
> I start to think I'm right. The raw data are completely unreadable by
> normal human being o you can't guess what is done when you need to fix
> the unittests^W code after some changes.
>
> IMHO, we should have on top of these raw data a textual representation
> of the byte fields in comment. Like this is done somewhere else in the
> app-layer-smtp.c file:
>
> int SMTPParserTest14(void)
> {
>    /* 220 mx.google.com ESMTP d15sm986283wfl.6<CR><LF> */
>     static uint8_t welcome_reply[] = {
>
> What do you think ?

I agree. Here's how I'm doing it for some of the DNP3 unit tests...

    uint8_t request[] = {
        /* Link header. */
        0x05, 0x64, 0x1a, 0xc4, 0x02, 0x00, 0x01, 0x00,

        /* CRC. */
        0xa5, 0xe9,

        /* Transport header. */
        0xff,

        /* Application layer. */
        0xc9, 0x05, 0x0c, 0x01, 0x28, 0x01, 0x00, 0x00,
        0x00, 0x01, 0x01, 0x01, 0x00, 0x00, 0x00,

        /* CRC. */
        0x72, 0xef,

        /* Application layer. */
        0x00, 0x00, 0x00, 0x00, 0x00,

        /* CRC. */
        0xff, 0xff,
    };

It will requires a quick glance to the struct sometimes to know whats
what, but much better than a big old blob of hex IMO.