string padding: incorrect length.

[LeeAlfay]

Hi,

The following method is copying a string to a SBE message buffer but the padding is not working for a string length less than 20. The loop that adds the padding (this part: start < length) needs to use the buffer size not the srcLength, no?

SBEOrderResponseMsg &putSymbol(const std::string& str)
{
    const size_t srcLength = str.length();
    if (srcLength > 20)
    {
         throw std::runtime_error("string too large for putSymbol [E106]");
    }

    size_t length = srcLength < 20 ? srcLength : 20;
    std::memcpy(m_buffer + m_offset + 25, str.c_str(), length);
    for (size_t start = srcLength; start < **length**; ++start)
    {
        m_buffer[m_offset + 25 + start] = 0;
    }

    return *this;
}

[mjpt777]

Fixed now. Thanks.

[LeeAlfay]

thanks.

[LeeAlfay]

Just one other suggestion on this topic...it might make sense in the getXXXAsString methods that are created to return a string WITHOUT the whitespace...

std::string getXXXAsString() const
{
    std::string result(m_buffer + m_offset + 28, 20);
    return result;
}

[mjpt777]

Done with this commit. 88a6f79

[tmontgomery]

There are use cases already where fixed length arrays are used that are not strings, but are raw data. And a string is used. I think since this is a fixed length array, it should be the size it is. Let the caller decide whether or not to trim, etc.

[mjpt777]

Now it is consistent with how the Java implementation has been for some time. With a string it does not feel right to have unprintable characters.

[tmontgomery]

It is a fairly common practice to store raw data in a std::string (however, I am not condoning it). And I know users who do with SBE.... this will break their usage. Java does things differently and so, makes sense for Java. But not for C++... and if we do break operation, this is not something that is easily seen nor tested for.

[mjpt777]

It is a tricky one. For supported customers we should do the right thing. From a design symmetry perspective I believe that if it null pads for short strings on encode then it should give a trimmed string on decode. Knowing that people we don't directly support do crazy things feels like the wrong reason to allow an asymmetry. If raw data then it should be an array of unit8. Maybe I'm just too picky.

[LeeAlfay]

Great, thanks for the fix. its an interesting discussion you are having. I tend to agree with the the comment "I believe that if it null pads for short strings on encode then it should give a trimmed string on decode". Otherwise its a little confusing with the function naming getXXAsString(). Anything, thanks for the fix!

…

On Mon, Mar 4, 2019 at 5:15 AM Martin Thompson ***@***.***> wrote: It is a tricky one. For supported customers we should do the right thing. From a design symmetry perspective I believe that if it null pads for short strings on encode then it should give a trimmed string on decode. Knowing that people we don't directly support do crazy things feels like the wrong reason to allow an asymmetry. If raw data then it should be an array of unit8. Maybe I'm just too picky. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#646 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ai60NImr7ZBjW53OjGeyZ87kaLa8gg37ks5vTC1NgaJpZM4bPoh3> .