Hi,

On Wed, Oct 17, 2018 at 10:33 AM Jonas Bonn <jonas@southpole.se> wrote:
>
> Hi,
>
>
> On 17/10/18 09:30, Giacinto Cifelli wrote:
>
> >>> +
> >>> +#ifdef HAVE_CONFIG_H
> >>> +#include <config.h>
> >>> +#endif
> >>> +
> >>> +#define _GNU_SOURCE
> >> What requires _GNU_SOURCE?
> > I don't know. It was part of drivers/atmodem/voicecall.c.
> > Maybe Denis knows?
>
> If you don't know that you need it, drop it.  If the code fails to build
> without it, you need it.  I can't immediately see what requires this in
> your code, but maybe there's something.  Otherwise it would be nice not
> to propagate this further when somebody cut-and-paste's your code. :)
>
> drivers/atmodem/voicecall.c doesn't need it either.  Builds fine without.

it builds without. I have dropped it.
I will push a patch for atmodem/voicecall too.

>
> >
> >>> +
> >>> +static struct ofono_voicecall_driver driver = {
> >>> +     .name                   = "gemaltomodem",
> >>> +     .probe                  = gemalto_voicecall_probe,
> >>> +     .remove                 = gemalto_voicecall_remove,
> >>> +     .dial                   = gemalto_dial,
> >>> +     .answer                 = gemalto_answer,
> >>> +     .hangup_all             = gemalto_hangup_all,
> >>> +     .hangup_active          = gemalto_hangup,
> >>> +     .hold_all_active        = gemalto_hold_all_active,
> >>> +     .release_all_held       = gemalto_release_all_held,
> >>> +     .set_udub               = gemalto_set_udub,
> >>> +     .release_all_active     = gemalto_release_all_active,
> >>> +     .release_specific       = gemalto_release_specific,
> >>> +     .private_chat           = gemalto_private_chat,
> >>> +     .create_multiparty      = gemalto_create_multiparty,
> >>> +     .transfer               = gemalto_transfer,
> >>> +     .deflect                = NULL,
> >>> +     .swap_without_accept    = NULL,
> >> Are other drivers explicit about _not_ providing implementations? I'd
> >> drop these otherwise?
> >>
> > again, it is done so in the original drivers/atmodem/voicecall.c.
>
> I would drop them...

I would like to know why they weren't dropped in atmodem in the first
place, before dropping them.
Either solution is ok for me, I just want some clarification.

>
> >
> > overall, thank you for reviewing and also for your questions, it gave
> > me the possibility to clarify some points.
> >
> > regards,
> > Giacinto
> > ---
> > full list of suspicious externs:
>
> Thanks.  Some of those "suspicious externs" are mine, too! :)

so we keep them?

>
> /Jonas
>

Giacinto