Fixed precision zero string printing and added tests for it

[warasilapm]

The C standard says that the precision field for a string sets the maximum number of bytes to print. This PR addresses the check for a buffer size of zero in prv_out_str() that breaks this guarantee. While it may seem nonsensical to have this behavior, I found this issue when I passed zero into the first field of "%.*s" and it printed the buffer passed in the second field. There are some cases where this sort of behavior may be helpful for strings terminated by non-null characters.

Please check the that the tests pass/fail as expected.

[CLAassistant]

All committers have signed the CLA.

[MaJerle]

You are right, but I think fix is not to remove if statement for buffer size, because if there is no precision, then we need to determine buffer size.

Solution would be rather to check if precision flag was set and only if not, then check for string length.

    if (buff_size == 0 && !p->m.flags.precision) {
        buff_size = strlen(buff);
    }

Try and let me know if it works for you. It does on my side.

Test code

    for (int i = 0; i < strlen("Text string 123"); ++i) {
        printf_run(NULL, "%.*s", i, "Text string 123");
    }

Outputs

Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: ""
Length VSprintf: 0
Result LwPRINTF: ""
Length LwPRINTF: 0
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "T"
Length VSprintf: 1
Result LwPRINTF: "T"
Length LwPRINTF: 1
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Te"
Length VSprintf: 2
Result LwPRINTF: "Te"
Length LwPRINTF: 2
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Tex"
Length VSprintf: 3
Result LwPRINTF: "Tex"
Length LwPRINTF: 3
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text"
Length VSprintf: 4
Result LwPRINTF: "Text"
Length LwPRINTF: 4
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text "
Length VSprintf: 5
Result LwPRINTF: "Text "
Length LwPRINTF: 5
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text s"
Length VSprintf: 6
Result LwPRINTF: "Text s"
Length LwPRINTF: 6
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text st"
Length VSprintf: 7
Result LwPRINTF: "Text st"
Length LwPRINTF: 7
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text str"
Length VSprintf: 8
Result LwPRINTF: "Text str"
Length LwPRINTF: 8
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text stri"
Length VSprintf: 9
Result LwPRINTF: "Text stri"
Length LwPRINTF: 9
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text strin"
Length VSprintf: 10
Result LwPRINTF: "Text strin"
Length LwPRINTF: 10
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text string"
Length VSprintf: 11
Result LwPRINTF: "Text string"
Length LwPRINTF: 11
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text string "
Length VSprintf: 12
Result LwPRINTF: "Text string "
Length LwPRINTF: 12
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text string 1"
Length VSprintf: 13
Result LwPRINTF: "Text string 1"
Length LwPRINTF: 13
Test result: Pass
----
Format: "%.*s"
Params: "i, "Text string 123""
Result VSprintf: "Text string 12"
Length VSprintf: 14
Result LwPRINTF: "Text string 12"
Length LwPRINTF: 14
Test result: Pass

[warasilapm]

Sorry, it was very late for me when I wrote that. I left out part of my reasoning for removing that check in particular. The check I removed is redundant. All calls in lwprintf.c to prv_out_str() either have a constant non-zero buff_size or in the case of the call from prv_format(), this check for the precision field has already been done in the caller.

// lwprintf.c:1066
case 's': {
    const char* b = va_arg(arg, const char*);
    size_t len = strlen(b);

    /* Precision gives maximum output len */
    if (p->m.flags.precision) {
        if (len > p->m.precision) {
            len = p->m.precision;
        }
    }
    prv_out_str(p, b, len);
    break;
}

[MaJerle]

Yes, but problem is that prv_out_str did an overwrite of the len if it was 0. Now adding check for flag, makes it working properly

    if (buff_size == 0 /* && !p->m.flags.precision <- this has been added */) {
        buff_size = strlen(buff);
    }

[warasilapm]

This is badness.

While I agree that the problem I experienced with prv_out_str() is specifically that it overwrote zero, the check being there is completely pointless. All of the calls to this private function are bounded already. There is now duplicate code in the tree for no benefit.

Issues with this approach:

1. Two places to maintain code.
2. Two excess runs of the same checks during normal operation of the code
3. If a zero is passed when the precision flag is not set, the code now runs an unbounded strlen() on a buffer that may or may not be valid.

[warasilapm]

I should add that the tests all pass with this redundant check removed.

[MaJerle]

I see your point now, sorry.

But there is still bug. If precision is provides, then string doesnt need to have null termination, so strlen must be fixed too

[warasilapm]

I apologize if that came across as rude. I call any questionable construct "badness".

I'll continue on the null termination question in #9