Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH 12/27] acd: add send_announce_packet
(Christian Spielberger)
----------------------------------------------------------------------
Message: 1
Date: Tue, 8 May 2018 22:41:06 +0200
From: Christian Spielberger <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 12/27] acd: add send_announce_packet
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
On Mon 16.04.18 16:56, Daniel Wagner wrote:
> Hi Christian,
>
> On 04/11/2018 04:00 PM, Christian Spielberger wrote:
> > Adding function for sending announce packets and only function headers of
> > the timeout callbacks acd_announce_timeout for repeated announce packets and
> > acd_defend_timeout for switching from DEFEND state back to MONITOR state.
>
> > +static gboolean send_announce_packet(gpointer acd_data)
> > +{
> > + acd_host *acd = acd_data;
> > +
> > + debug(acd, "sending ACD announce request");
> > +
> > + send_arp_packet(acd->mac_address,
> > + acd->requested_ip,
> > + acd->requested_ip,
> > + acd->ifindex);
> > +
> > + remove_timeout(acd);
> > +
> > + if (acd->state == ACD_DEFEND)
> > + acd->timeout = g_timeout_add_seconds_full(G_PRIORITY_HIGH,
> > + DEFEND_INTERVAL,
> > + acd_defend_timeout,
> > + acd,
> > + NULL);
> > + else
> > + acd->timeout = g_timeout_add_seconds_full(G_PRIORITY_HIGH,
> > + ANNOUNCE_INTERVAL,
> > + acd_announce_timeout,
> > + acd,
> > + NULL);
>
> This looks a bit ugly. What about something like
>
> interval = acd->state == ACD_DEFEND? DEFEND_INTERVAL : ANNOUCE_INTERVAL;
> acd->timeout = g_timeout_add_seconds_full(G_PRIORITY_HIGH,
> interval,
> acd_announce_timeout,
> acd,
> NULL);
>
> Thanks,
> Daniel
Hello Daniel,
I have this from gdhcp/client.c function send_announce_packet. (Yes, after
the ACD patches are applied, we have to refactor this.)
The function send_announce_packet has to handle two different states (it
is a state machine). It handles state ACD_DEFEND and ACD_ANNOUNCE. Not only
the delay is different also the callback. The if-else is a better visualization
for these two states. Even better would be a switch-case statement.
But if you prefer yours, I could use a function pointer for the timeout
callback. Like this:
interval = acd->state == ACD_DEFEND? DEFEND_INTERVAL : ANNOUCE_INTERVAL;
callback = acd->state == ACD_DEFEND? acd_defend_timeout :
acd_announce_timeout;
acd->timeout = g_timeout_add_seconds_full(G_PRIORITY_HIGH,
interval,
callback,
acd,
NULL);
Two more variables. Really?
But it is your decision. What do you prefer?
- if-else, like it is currently,
- switch-case, for even better state handling visualization,
- or the interval-callback solution?
regards,
christian.
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 31, Issue 5
**************************************
