Hey, On Sat, Dec 16, 2017 at 3:03 AM, Ben Chan <benc...@chromium.org> wrote: > On Fri, Dec 15, 2017 at 1:16 AM Aleksander Morgado wrote: > > Probably a > good change; will test it once I have a chance. > > But regarding the last > fallback, why is it better to return an empty > string than NULL? I assume > that we return NULL to indicate an error in > parsing, which is what happens > here if we cannot do UCS2 or UTF-16 > translations. My initial thought was > to match how the code handles the case when encoding isn't GSM7 or UCS-2, > i.e. } else { g_warn_if_reached (); utf8 = g_strdup (""); } But if the > following warning is preferred, I can undo this change. > 2017-12-16T10:53:58.844064+09:00 WARNING ModemManager[29345]: > (mm-sms-part-3gpp.c:701):mm_sms_part_3gpp_new_from_binary_pdu: runtime check > failed: (mm_sms_part_get_text (sms_part) != NULL)
I would actually prefer an explicit warning about the issue, e.g. "couldn't convert SMS data to text". Maybe even specifying what the actual data was in an hex printed string, e.g. "couldn't convert SMS data to text: 0xab,0xcd,0xde...". What do you think? This is something we could do for every other case of failing character set conversions. Either way, that would probably be better in a follow-up patch I guess. -- Aleksander https://aleksander.es _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel