The release buildtype will disable certain warnings that we do see in
debug builds. Ensure we have a test build with all features enabled in
debug mode.

Selecting the build type as release limits the amount of warnings that
are enabled, so ensure we always build with warnings treated as errors
so that we don't miss any warning that would happen on debug builds.

  ../src/plugins/cinterion/mm-plugin-cinterion.c:65:1: warning: no previous prototype for ‘is_port_already_tagged’ [-Wmissing-prototypes]
     65 | is_port_already_tagged (MMPortProbe *probe)
        | ^~~~~~~~~~~~~~~~~~~~~~

Konrad Zapałowicz authored 2 years ago and Aleksander Morgado committed 1 year ago

The Cinterion plugin is using the output of the SQPORT? to guess which
ports can be used for AT commands and data connection.

Yet at the same time the udev adds port type hints to the Cinterion
modem upon its discovery.

This commit changes the initialization in a way that from now on it
skips sending SQPORT? when the port type hints are already assigned. By
doing this we make sure that the udev port type hints are being used
when they are available. In case they are not the initialization relies
on the outout of SQPORT? as it did do far.

See: mobile-broadband/ModemManager!782

Lubomir Rintel authored 1 year ago and Aleksander Morgado committed 1 year ago

GCC 13 got unhappy about using an int in place of an enum:

  ../src/mm-port-qmi.c:241:1: warning: conflicting types for ‘mm_port_qmi_release_client’ due to enum/integer mismatch; have ‘void(MMPortQmi *, QmiService,  MMPortQmiFlag)’ {aka ‘void(struct _MMPortQmi *, QmiService,  MMPortQmiFlag)’} [-Wenum-int-mismatch]
    241 | mm_port_qmi_release_client (MMPortQmi     *self,
        | ^~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from ../src/mm-port-qmi.c:26:
  ../src/mm-port-qmi.h:113:10: note: previous declaration of ‘mm_port_qmi_release_client’ with type ‘void(MMPortQmi *, QmiService,  guint)’ {aka ‘void(struct _MMPortQmi *, QmiService,  unsigned int)’}
    113 | void     mm_port_qmi_release_client         (MMPortQmi  *self,
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

Lubomir Rintel authored 1 year ago and Aleksander Morgado committed 1 year ago

GCC 13 is unhappy to cast a MMLogLevel to a guint32:

  [188/539] Compiling C object src/plugins/test-shared-icera.p/icera_tests_test-modem-helpers-icera.c.o
  In file included from ../src/plugins/icera/tests/test-modem-helpers-icera.c:27:
  ../src/mm-log-test.h:25:1: warning: conflicting types for ‘_mm_log’ due to enum/integer mismatch; have ‘void(void *, const gchar *, const gchar *, const gchar *, guint32,  const gchar *, ...)’ {aka ‘void(void *, const char *, const char *, const char *, unsigned int,  const char *, ...)’} [-Wenum-int-mismatch]
     25 | _mm_log (gpointer     obj,
        | ^~~~~~~
  In file included from ../src/mm-log-test.h:20:
  ../src/mm-log.h:61:6: note: previous declaration of ‘_mm_log’ with type ‘void(void *, const gchar *, const gchar *, const gchar *, MMLogLevel,  const gchar *, ...)’ {aka ‘void(void *, const char *, const char *, const char *, MMLogLevel,  const char *, ...)’}
     61 | void _mm_log (gpointer     obj,
        |      ^~~~~~~

That's perfectly fine, just use the enum type directly.

Lubomir Rintel authored 1 year ago and Aleksander Morgado committed 1 year ago

GCC 13 is unhappy about mixing enums and ints. However, if we fix the
type un mmtty's _mm_log() prototype, the compiler will find something
else to be irritiated about:

  [1/2] Compiling C object test/mmtty.p/mmtty.c.o
  ../test/mmtty.c: In function ‘_mm_log’:
  ../test/mmtty.c:283:5: warning: enumeration value ‘MM_LOG_LEVEL_MSG’ not handled in switch [-Wswitch-enum]
    283 |     switch (level) {
        |     ^~~~~~
  [2/2] Linking target test/mmtty

Fix that first.

  ==174467== Invalid read of size 1
  ==174467==    at 0x10B80C: read_bits (mm-sms-part-cdma.c:255)
  ==174467==    by 0x10B886: read_bits (mm-sms-part-cdma.c:260)
  ==174467==    by 0x10DC2F: read_bearer_data_user_data (mm-sms-part-cdma.c:882)
  ==174467==    by 0x10DC2F: read_bearer_data (mm-sms-part-cdma.c:1000)
  ==174467==    by 0x10DC2F: mm_sms_part_cdma_new_from_binary_pdu (mm-sms-part-cdma.c:1180)
  ==174467==    by 0x10DF24: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:331)
  ==174467==    by 0x10A91D: common_test_valid_part_from_hexpdu (test-sms-part-cdma.c:114)
  ==174467==    by 0x10B0AC: common_test_valid_part_from_pdu (test-sms-part-cdma.c:126)
  ==174467==    by 0x10B0AC: test_invalid_ascii_user_data (test-sms-part-cdma.c:412)
  ==174467==    by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==  Address 0x51a6457 is 0 bytes after a block of size 7 alloc'd
  ==174467==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==174467==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==174467==    by 0x10DF06: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:325)
  ==174467==    by 0x10A91D: common_test_valid_part_from_hexpdu (test-sms-part-cdma.c:114)
  ==174467==    by 0x10B0AC: common_test_valid_part_from_pdu (test-sms-part-cdma.c:126)
  ==174467==    by 0x10B0AC: test_invalid_ascii_user_data (test-sms-part-cdma.c:412)
  ==174467==    by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)

Same fix also applied to latin encoded text as it also makes sense there.

  ==158856== Invalid read of size 1
  ==158856==    at 0x10B814: read_bits (mm-sms-part-cdma.c:257)
  ==158856==    by 0x10DB07: read_bearer_data_user_data (mm-sms-part-cdma.c:878)
  ==158856==    by 0x10DB07: read_bearer_data (mm-sms-part-cdma.c:990)
  ==158856==    by 0x10DB07: mm_sms_part_cdma_new_from_binary_pdu (mm-sms-part-cdma.c:1170)
  ==158856==    by 0x10DE54: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:333)
  ==158856==    by 0x10A916: common_test_invalid_part_from_hexpdu (test-sms-part-cdma.c:90)
  ==158856==    by 0x10A916: common_test_invalid_part_from_pdu (test-sms-part-cdma.c:104)
  ==158856==    by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A02BBC: g_test_run (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x10A509: main (test-sms-part-cdma.c:595)
  ==158856==  Address 0x51a627b is 0 bytes after a block of size 11 alloc'd
  ==158856==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==158856==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==158856==    by 0x10DE36: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:327)
  ==158856==    by 0x10A916: common_test_invalid_part_from_hexpdu (test-sms-part-cdma.c:90)
  ==158856==    by 0x10A916: common_test_invalid_part_from_pdu (test-sms-part-cdma.c:104)
  ==158856==    by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==    by 0x4A02BBC: g_test_run (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==158856==

  GLib-CRITICAL **: 21:21:51.419: g_convert: assertion 'str != NULL' failed

  Program received signal SIGTRAP, Trace/breakpoint trap.
  0x00007ffff7db3e82 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  (gdb) bt
  #0  0x00007ffff7db3e82 in g_logv () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #1  0x00007ffff7db40ef in g_log () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #2  0x00007ffff7d8a5da in g_convert () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #3  0x00005555555592cf in read_bearer_data_user_data (log_object=0x0, subparameter=<optimized out>, sms_part=0x555555578000)
      at ../src/mm-sms-part-cdma.c:929
  #4  read_bearer_data (log_object=0x0, parameter=<optimized out>, sms_part=0x555555578000) at ../src/mm-sms-part-cdma.c:982
  #5  mm_sms_part_cdma_new_from_binary_pdu

  ERROR:../src/mm-sms-part-cdma.c:678:read_cause_codes: assertion
  failed: (parameter->parameter_id == PARAMETER_ID_BEARER_REPLY_OPTION)

  [debug] parsing PDU (0)...
  [debug]   no SMSC address given
  [debug]   submit type PDU detected
  [debug]   message reference: 1
  [debug]   address size: 1 digits (1 bytes)
  [debug]   number parsed: 00
  [debug]   validity available, format relative
  [debug]   PID: 0
  [debug]   user data encoding is GSM7
  [debug]   user data length: 0 elements
  [debug]   user data length: 0 bytes

  ==125780== Command: ./build/test/mmsmspdu --pdu=00F101010C0000000000 --verbose
  ==125780==
  ==125780== Invalid read of size 1
  ==125780==    at 0x10B422: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:698)
  ==125780==    by 0x10BF57: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
  ==125780==    by 0x10A44D: main (mmsmspdu.c:242)
  ==125780==  Address 0x519988a is 0 bytes after a block of size 10 alloc'd
  ==125780==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==125780==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==125780==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==125780==    by 0x10BF36: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
  ==125780==    by 0x10A44D: main (mmsmspdu.c:242)

  ==101461== Command: ./build/test/mmsmspdu --pdu=004100010100014B00002E --verbose
  ==101461==
  [debug] parsing PDU (0)...
  [debug]   no SMSC address given
  [debug]   submit type PDU detected
  [debug]   message reference: 0
  [debug]   address size: 1 digits (1 bytes)
  [debug]   number parsed: 00
  [debug]   PID: 1
  [debug]   user data encoding is GSM7
  [debug]   user data length: 0 elements
  [debug]   user data length: 0 bytes
  [debug] decoding SMS text with 4294967294 elements

Based on a patch from Michal Mazur <mkm@semihalf.com>.

Before the actual number digits there is always a Type of Address byte
that we were not considering during the size check.

  [debug] parsing PDU (0)...
  [debug]   no SMSC address given
  [debug]   deliver type PDU detected
  [debug]   address size: 1 digits (1 bytes)

  ==90832== Command: ./build/test/mmsmspdu --pdu=001C011C --verbose
  ==90832==
  ==90832== Invalid read of size 1
  ==90832==    at 0x10AC90: sms_semi_octets_to_bcd_string (mm-sms-part-3gpp.c:71)
  ==90832==    by 0x10AC90: sms_decode_address (mm-sms-part-3gpp.c:157)
  ==90832==    by 0x10B0C5: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:512)
  ==90832==    by 0x10BF77: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
  ==90832==    by 0x10A44D: main (mmsmspdu.c:242)
  ==90832==  Address 0x5199874 is 0 bytes after a block of size 4 alloc'd
  ==90832==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==90832==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==90832==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==90832==    by 0x10BF56: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
  ==90832==    by 0x10A44D: main (mmsmspdu.c:242)

    [debug] parsing PDU (0)...
    [debug]   no SMSC address given
    [debug]   status report type PDU detected
    [debug]   message reference: 191
    [debug]   address size: 0 digits (0 bytes)

  ==78906== Command: ./build/test/mmsmspdu --pdu=000ABF00 --verbose
  ==78906==
  ==78906== Invalid read of size 1
  ==78906==    at 0x10AA80: sms_decode_address (mm-sms-part-3gpp.c:132)
  ==78906==    by 0x10AF7C: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:507)
  ==78906==    by 0x10BE17: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
  ==78906==    by 0x10A44D: main (mmsmspdu.c:202)
  ==78906==  Address 0x5199874 is 0 bytes after a block of size 4 alloc'd
  ==78906==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==78906==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==78906==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==78906==    by 0x10BDF6: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
  ==78906==    by 0x10A44D: main (mmsmspdu.c:202)

Michał Mazur authored 1 year ago and Aleksander Morgado committed 1 year ago

Always prefer the operation error when the nw_error is not set (has
value MBIM_NW_ERROR_NONE). If the activation state is ACTIVATED or
ACTIVATING then the behavior doesn't change.

Aleksander Morgado authored 1 year ago and Aleksander Morgado committed 1 year ago

When QCDM is not required we don't run an explicit QCDM port probing
operation.

In this case, though, we should not assume that the port is QCDM
capable, even if it is also flagged as ignored.

Instead, we'll flag the port as QCDM capable and ignored only if there
was a udev port type hint associated to the port. Otherwise, we'll
report the port as not being QCDM capable, and the port won't even be
reported in the list of ports as its type is unknown.

The supported Quectel modules are usually QMI based and therefore
very new, there is no real requirement to support QCDM based
management.

The supported broadmobi modules are usually QMI based and therefore
very new, there is no real requirement to support QCDM based
management.

The QCDM ports are not strictly required in the Sierra plugin, as this
plugin deals exclusively with QMI and MBIM capable devices.

Aleksander Morgado authored 1 year ago and Aleksander Morgado committed 1 year ago

Happens when we boot without a SIM card inserted:

  ModemManager[12065]: <dbg> [1679912011.887410] [ttyACM2/at] --> 'AT+CLCK="PC",2<CR>'
  ModemManager[12065]: <dbg> [1679912011.905401] [ttyACM2/at] <-- '<CR><LF>+CLCK: 0<CR><LF><CR><LF>OK<CR><LF>'
  (ModemManager:12065): GLib-GObject-CRITICAL **: 12:13:31.905: invalid (NULL) pointer instance

Akash Aggarwal authored 1 year ago and Aleksander Morgado committed 1 year ago

Fixes mobile-broadband/ModemManager#646

Aleksander Morgado authored 1 year ago and Aleksander Morgado committed 1 year ago

The feature/rx_offload and feature/tx_offload sysfs attributes specify
which offload settings should be used when creating new links.

Aleksander Morgado authored 1 year ago and Aleksander Morgado committed 1 year ago

This reverts commit dad3e827.

Disabling profile change indications just before doing our profile
updates and re-enabling them just after is not enough. The modem may
still emit the profile change indication right after, so all this
logic is unnecessary.

Kirill Buksha authored 1 year ago and Aleksander Morgado committed 1 year ago

GError structure may not be initialized after execution of
mm_3gpp_parse_cgdcont_read_response() and accessing it's
fields will cause a segmentation fault.

  src/mm-iface-modem-signal.c:99:27: warning: unused variable 'current_time' [-Wunused-variable]
      g_autoptr(GDateTime)  current_time = NULL;
                            ^

  src/mm-sim-mbim.c:1134:30: warning: unused variable 'request' [-Wunused-variable]
      g_autoptr(MbimMessage)   request = NULL;
                               ^