[PATCH 1/3] fix code generation for emit_size_read() to check for buffer overflow

[Aleksander Morgado]

On Wed, Oct 8, 2014 at 10:00 AM, Thomas Haller <thaller at redhat.com> wrote:
>
>> The code doesn't return a GError in validate(), just does a g_warning,
>> because we don't want to halt the full message parsing if we detect a
>> TLV with wrong contents. An incorrect TLV, or just a TLV which we
>> don't know how to parse, shouldn't make the parsing of other TLVs
>> break. validate() in this case is done for each TLV while we parse the
>> received message; and we do an initial parsing of the whole message by
>> default.
>>
>> Another, maybe better, approach would be to only parse those TLVs that
>> the user wants. i.e. the 'output' bundle would only fill in the parsed
>> fields when a given TLV is requested. And in that case, if a single
>> TLV is requested from the output bundle, we could then run validate()
>> and return a GError. IIRC this is more or less what I did in libmbim
>> actually, where the result of a client operation is not an already
>> parsed output bundle, but the actual message itself, and in that case
>> only the user-requested TLVs get parsed and built.
>
> The validation code uses g_warning() in case of failure. I think that is
> not a great way to report runtime errors/warnings. I think it's suitable
> to warn about bugs.
>
> One reason is that I find it useful to run applications with
> G_DEBUG=fatal-warnings. Which does not work if the application (or even
> one of it's libraries) uses g_warning for non-bugs.
>
> I think a library should not print anything (or possibly only when
> having a QMI_DEBUG variable defined).

All libqmi logging is already done under the "Qmi" G_LOG_DOMAIN; so
programs using libqmi should only get logs printed if a log handler
was explicitly set for that log domain.

-- 
Aleksander
https://aleksander.es