Re: [PATCH] [MM 0.7] Derive PDP type from the ip-type bearer property

[Tore Anderson <tore fud no>]

Hi,

* Aleksander Morgado

> I'm guessing that you removed the g_free() before because it was
> crashing afterwards when you tried to g_free() this "IP" string that
> you're setting here. In order to avoid that, g_free() should receive
> always a string allocated in heap:
>  self->priv->ip_type = g_strdup ("IP");

Yep, your guess is absolutely correct. Thanks for the suggested fix,
will implement.

>> @@ -758,8 +758,9 @@ find_cid_ready (MMBaseModem *modem,
>>
>>       /* Initialize PDP context with our APN */
>>       ctx->cid = g_variant_get_uint32 (result);
>> -    command = g_strdup_printf ("+CGDCONT=%u,\"IP\",\"%s\"",
>> +    command = g_strdup_printf ("+CGDCONT=%u,\"%s\",\"%s\"",
>>                                  ctx->cid,
>> +                               mm_bearer_properties_get_ip_type
>> (mm_bearer_peek_config (MM_BEARER (ctx->self))),
>>                                  mm_bearer_properties_get_apn
>> (mm_bearer_peek_config (MM_BEARER (ctx->self))));
> 
> There's some misalignment here, probably coming from the previous source
> code; could you fix that while you're at it?

This looked fine in the patch I sent and also in the mailing list
archives:

http://mail.gnome.org/archives/networkmanager-list/2012-April/msg00033.html

In any case, will send the updated patch as an attachment, hopefully
that's better.

> It seems you're comparing pdp->pdp_type with the pdp_type retrieved from
> the bearer config twice here (you already did it some lines before in
> the previous g_str_equal()). So no need for the pdp_type variable all
> together.

Doh! Indeed.

I will re-send an updated patch in a few minutes. Thank you for the
review and suggestions!

Best regards,
-- 
Tore Anderson