[PATCH] qmi-codegen: don't mark missing optional TLVs as set

Dan Williams dcbw at redhat.com
Fri Nov 21 11:33:32 PST 2014


In the optional TLV case, 'tlv_error' was never set to TRUE
when the optional TLV was missing, because the return value of
qmi_message_tlv_read_init() was ignored.  This caused
"self->arg_*_set" to always be TRUE and NULL strings to be
returned to callers requesting the TLV value later.

Also prevent incorrect "Left X bytes unread when getting..."
messages caused when the TLV is missing.

This bug was found when probing a modem that does not return
an MEID TLV to DMSGetIds (because it is GSM/UMTS/LTE only),
but qmi_message_dms_get_ids_output_get_meid() returned TRUE
and a NULL 'str':

    if (qmi_message_dms_get_ids_output_get_meid (output, &str, NULL) &&
-->     str[0] != '\0' && str[0] != '0') {

Bug introduced in b143b7f6 (qmi-codegen: use the new TLV reader API).

Before:
=======

    gsize offset = 0;
    gsize init_offset;
    gboolean tlv_error = FALSE;

    init_offset = qmi_message_tlv_read_init (message, QMI_INDICATION_DMS_EVENT_REPORT_OUTPUT_TLV_POWER_STATE, NULL, NULL);
    <<<snip>>>

    /* The remaining size of the buffer needs to be 0 if we successfully read the TLV */
    if ((offset = __qmi_message_tlv_read_remaining_size (message, init_offset, offset)) > 0) {
        g_warning ("Left '%" G_GSIZE_FORMAT "' bytes unread when getting the 'Power State' TLV", offset);
    }

qmi_indication_dms_event_report_output_power_state_out:
    if (!tlv_error)
        self->arg_power_state_set = TRUE;

After:
======

    gsize offset = 0;
    gsize init_offset;

    if ((init_offset = qmi_message_tlv_read_init (message, QMI_INDICATION_DMS_EVENT_REPORT_OUTPUT_TLV_POWER_STATE, NULL, NULL)) == 0) {
        goto qmi_indication_dms_event_report_output_power_state_out;
    }
    <<<snip>>>

    /* The remaining size of the buffer needs to be 0 if we successfully read the TLV */
    if ((offset = __qmi_message_tlv_read_remaining_size (message, init_offset, offset)) > 0) {
        g_warning ("Left '%" G_GSIZE_FORMAT "' bytes unread when getting the 'Power State' TLV", offset);
    }

    self->arg_power_state_set = TRUE;

qmi_indication_dms_event_report_output_power_state_out:
    ;

---
 build-aux/qmi-codegen/Field.py | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/build-aux/qmi-codegen/Field.py b/build-aux/qmi-codegen/Field.py
index 7d51b93..c64bf29 100644
--- a/build-aux/qmi-codegen/Field.py
+++ b/build-aux/qmi-codegen/Field.py
@@ -280,23 +280,25 @@ class Field:
         template = (
             '${lp}gsize offset = 0;\n'
             '${lp}gsize init_offset;\n'
-            '${lp}gboolean tlv_error = FALSE;\n'
             '\n'
-            '${lp}init_offset = qmi_message_tlv_read_init (message, ${tlv_id}, NULL, ${error});\n')
+            '${lp}if ((init_offset = qmi_message_tlv_read_init (message, ${tlv_id}, NULL, ${error})) == 0) {\n')
+
         if self.mandatory:
             template += (
-                '\n'
-                '${lp}if (init_offset == 0) {\n'
                 '${lp}    g_prefix_error (${error}, "Couldn\'t get the mandatory ${name} TLV: ");\n'
                 '${lp}    ${container_underscore}_unref (self);\n'
-                '${lp}    return NULL;\n'
-                '${lp}}\n')
+                '${lp}    return NULL;\n')
+        else:
+            template += (
+                '${lp}    goto ${tlv_out};\n')
 
+        template += (
+            '${lp}}\n')
 
         f.write(string.Template(template).substitute(translations))
 
         # Now, read the contents of the buffer into the variable
-        self.variable.emit_buffer_read(f, line_prefix + '    ', tlv_out, error, 'self->' + self.variable_name)
+        self.variable.emit_buffer_read(f, line_prefix, tlv_out, error, 'self->' + self.variable_name)
 
         template = (
             '\n'
@@ -305,15 +307,18 @@ class Field:
             '${lp}    g_warning ("Left \'%" G_GSIZE_FORMAT "\' bytes unread when getting the \'${name}\' TLV", offset);\n'
             '${lp}}\n'
             '\n'
-            '${tlv_out}:\n'
-            '${lp}if (!tlv_error)\n'
-            '${lp}    self->${variable_name}_set = TRUE;\n')
+            '${lp}self->${variable_name}_set = TRUE;\n'
+            '\n'
+            '${tlv_out}:\n')
         if self.mandatory:
             template += (
-                '${lp}else {\n'
+                '${lp}if (!self->${variable_name}_set) {\n'
                 '${lp}    ${container_underscore}_unref (self);\n'
                 '${lp}    return NULL;\n'
                 '${lp}}\n')
+        else:
+            template += (
+                '${lp};\n')
         f.write(string.Template(template).substitute(translations))
 
 
-- 
1.9.3




More information about the libqmi-devel mailing list