Hi Marcel.
Marcel Holtmann wrote:
> Hi Sjur,
>
>> diff --git a/drivers/stemodem/gprs-context.c
>> b/drivers/stemodem/gprs-context.c index c178fb6..79e697b 100644
>> --- a/drivers/stemodem/gprs-context.c
>> +++ b/drivers/stemodem/gprs-context.c
>> @@ -187,9 +187,11 @@ static gboolean caif_if_create(const char
>> *interface, unsigned int connid) return FALSE; }
>>
>> - if (ioctl(s, SIOCCAIFNETNEW, &ifr) < 0) {
>> - ofono_debug("Failed to create IP interface for CAIF");
>> - return FALSE;
>> + if (ioctl(s, SIOCGIFINDEX, &ifr) != 0) {
>> + if (ioctl(s, SIOCCAIFNETNEW, &ifr) < 0) {
>> + ofono_debug("Failed to create IP interface for CAIF");
>> + return FALSE; + }
>> }
>>
>> return TRUE;
>
> just doing it like this would be better:
>
> /* Check if interface exists */
> if (ioctl(s, SIOCGIFINDEX, &ifr) == 0)
> return TRUE;
>
> if (ioctl(s, SIOCCAIFNETNEW, &ifr) < 0) {
> ...
> return FALSE;
> }
>
> return TRUE;
>
> We are not a big fan complicated if clauses if they can be avoid with
> a simple goto or just a return.
>
> Also I think we need to check errno value here. Since potentially the
> ioctl can fail for other reasons. And maybe extending CAIF with a
> proper way of checking for an existing interface might be better.
Do you have anything particular in mind here?
Maybe CAIF could simply return EEXIST from SIOCCAIFNETNEW if the interface
already exists? In this case we could return TRUE anyway.
BR/Sjur
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
