>> sequence = g_strdup_printf ("%s%s%s", >> previous, >> @@ -118,7 +121,14 @@ gboolean >> mm_location_gps_nmea_add_trace (MMLocationGpsNmea *self, >> const gchar *trace) >> { >> - return location_gps_nmea_take_trace (self, g_strdup (trace)); >> + char *trace_copy; >> + >> + trace_copy = g_strdup (trace); >> + if (location_gps_nmea_take_trace (self, trace_copy)) >> + return TRUE; >> + >> + g_free (trace_copy); >> + return FALSE; >> } > > Since take_trace() is an internal function, and the current callers > don't do anything useful with a FALSE return, maybe we should just > update take_trace() to consume the input regardless of the return? > Otherwise callers will have some strings consumed, and others not > consumed, and the caller has to track that. Seems kinda pointless. > > > I was wondering the same thing, but not sure what's the typical semantic or > convention for something called *_take_*(). I would prefer a simpler > semantic, like you said, that it always take the ownership of the argument. > But if there is an established semantic or convention, I would be hesitated > to change it but that wouldn't prevent us from naming the function > differently ;) >
Oh, the convention for something called take_() should be that the ownership of the variable is transferred, so totally agree that the method should free() it internally. -- Aleksander https://aleksander.es _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel