AVRO-4246: [c] Fix memory leak on failed decoding

[fabiocfabini]

What is the purpose of the change

This pull request fixes a memory leak issue with the C SDK. Internal allocated buffer is never released if decoding fails.

Verifying this change

This change added test test_avro_4246.c that:

• encodes an integer into avro binary.
• attempts to decode into a string.
• valgrind catches whether memory as leaked internally.

Documentation

No new features are added.

[RyanSkraba]

Thanks for this fix! I'm not a C expert, so I wanted to clarify the intended behaviour here. When read_bytes() (or read_string()) returns an error, what should callers expect to find in the *bytes or *s pointer? Will it be NULL because of macro expansion or not?

[fabiocfabini]

Thank you for the feedback!

read_bytes() and read_string() are not part of the user API. They are called by the avro_value_read() which is part of the user API.

The stack frame goes like this: user frame -> avro_value_read() frame -> avro_read() frame.

avro_read dispatches to the proper read procedure depending on the inner type of the value being constructed. In the dispatch branches that relate to avro_bytes() and avro_read() these calls are wrapped inside a check_prefix (see read_bytes and read_string relevant source code lines) macro which short circuits in case of bad return code. On error, the pointers are not used.

So to answer your questions:

what should callers expect to find in the *bytes or *s pointer?: Given that the pointers are not used, they don't need to be in any specific state. However, assigning the pointers to NULL is standard.

Will it be NULL because of macro expansion or not?: Yes, in case of error the pointers are freed and set to NULL.