walter, check current git, it's a bit different there
On Thu, Jan 19, 2017 at 1:55 PM, walter harms <wha...@bfs.de> wrote: > > > Am 06.01.2017 12:13, schrieb Natanael Copa: >> Run the namelookup from the main loop so a misspelled first ntp server >> name does not block everything forever. >> >> This fixes the following situation which would block forever: >> $ sudo ./busybox ntpd -dn -p foobar -p pool.ntp.org >> ntpd: bad address 'foobar' >> ntpd: bad address 'foobar' >> ntpd: bad address 'foobar' >> ... >> >> This patch is based on Kaarle Ritvanens work. >> http://lists.busybox.net/pipermail/busybox/2016-May/084197.html >> >> function old new delta >> static.set_next - 23 +23 >> step_time 426 422 -4 >> recv_and_process_peer_pkt 2342 2338 -4 >> ntpd_main 1178 1171 -7 >> add_peers 229 217 -12 >> resolve_peer_hostname 102 88 -14 >> .rodata 125081 125065 -16 >> ------------------------------------------------------------------------------ >> (add/remove: 1/0 grow/shrink: 0/6 up/down: 23/-57) Total: -34 >> bytes >> text data bss dec hex filename >> 802129 15459 2192 819780 c8244 busybox_old >> 802095 15459 2192 819746 c8222 busybox_unstripped >> >> Signed-off-by: Kaarle Ritvanen <kaarle.ritva...@datakunkku.fi> >> Signed-off-by: Natanael Copa <nc...@alpinelinux.org> >> --- >> networking/ntpd.c | 71 >> ++++++++++++++++++++++++++----------------------------- >> 1 file changed, 33 insertions(+), 38 deletions(-) >> >> diff --git a/networking/ntpd.c b/networking/ntpd.c >> index b7fa5dce9..97e3919fb 100644 >> --- a/networking/ntpd.c >> +++ b/networking/ntpd.c >> @@ -155,6 +155,7 @@ >> #define RETRY_INTERVAL 32 /* on send/recv error, retry in N secs >> (need to be power of 2) */ >> #define NOREPLY_INTERVAL 512 /* sent, but got no reply: cap next query >> by this many seconds */ >> #define RESPONSE_INTERVAL 16 /* wait for reply up to N secs */ >> +#define HOSTNAME_INTERVAL 4 /* hostname lookup failed. Wait N secs for >> next try */ >> >> /* Step threshold (sec). std ntpd uses 0.128. >> */ >> @@ -293,6 +294,7 @@ typedef struct { >> >> typedef struct { >> len_and_sockaddr *p_lsa; >> + char *p_hostname; >> char *p_dotted; >> int p_fd; >> int datapoint_idx; >> @@ -318,7 +320,6 @@ typedef struct { >> datapoint_t filter_datapoint[NUM_DATAPOINTS]; >> /* last sent packet: */ >> msg_t p_xmt_msg; >> - char p_hostname[1]; >> } peer_t; >> >> >> @@ -791,27 +792,17 @@ reset_peer_stats(peer_t *p, double offset) >> } >> >> static void >> -resolve_peer_hostname(peer_t *p, int loop_on_fail) >> -{ >> - len_and_sockaddr *lsa; >> - >> - again: >> - lsa = host2sockaddr(p->p_hostname, 123); >> - if (!lsa) { >> - /* error message already emitted by host2sockaddr() */ >> - if (!loop_on_fail) >> - return; >> -//FIXME: do this to avoid infinite looping on typo in a hostname? >> -//well... in which case, what is a good value for loop_on_fail? >> - //if (--loop_on_fail == 0) >> - // xfunc_die(); >> - sleep(5); >> - goto again; >> +resolve_peer_hostname(peer_t *p) { >> + len_and_sockaddr *lsa = host2sockaddr(p->p_hostname, 123); >> + if (lsa) { >> + if (p->p_lsa) { >> + free(p->p_lsa); >> + free(p->p_dotted); >> + } > free will accept NULL so you can drop the if, or is there an other problem ? > >> + p->p_lsa = lsa; >> + p->p_dotted = xmalloc_sockaddr2dotted_noport(&lsa->u.sa); >> } >> - free(p->p_lsa); >> - free(p->p_dotted); >> - p->p_lsa = lsa; >> - p->p_dotted = xmalloc_sockaddr2dotted_noport(&lsa->u.sa); >> + set_next(p, lsa ? 0 : HOSTNAME_INTERVAL); > > is it possible to move the set_next into the if(lsa) block above > i feel that would improve readability. > > re, > wh > >> } >> >> static void >> @@ -820,28 +811,29 @@ add_peers(const char *s) >> llist_t *item; >> peer_t *p; >> >> - p = xzalloc(sizeof(*p) + strlen(s)); >> - strcpy(p->p_hostname, s); >> - resolve_peer_hostname(p, /*loop_on_fail=*/ 1); >> + p = xzalloc(sizeof(*p)); >> + p->p_hostname = xstrdup(s); >> + resolve_peer_hostname(p); >> >> /* Names like N.<country2chars>.pool.ntp.org are randomly resolved >> * to a pool of machines. Sometimes different N's resolve to the same >> IP. >> * It is not useful to have two peers with same IP. We skip duplicates. >> */ >> - for (item = G.ntp_peers; item != NULL; item = item->link) { >> - peer_t *pp = (peer_t *) item->data; >> - if (strcmp(p->p_dotted, pp->p_dotted) == 0) { >> - bb_error_msg("duplicate peer %s (%s)", s, p->p_dotted); >> - free(p->p_lsa); >> - free(p->p_dotted); >> - free(p); >> - return; >> + if (p->p_lsa) >> + for (item = G.ntp_peers; item != NULL; item = item->link) { >> + peer_t *pp = (peer_t *) item->data; >> + if (pp->p_lsa && strcmp(p->p_dotted, pp->p_dotted) == >> 0) { >> + bb_error_msg("duplicate peer %s (%s)", s, >> p->p_dotted); >> + free(p->p_hostname); >> + free(p->p_lsa); >> + free(p->p_dotted); >> + free(p); >> + return; >> + } >> } >> - } >> >> p->p_fd = -1; >> p->p_xmt_msg.m_status = MODE_CLIENT | (NTP_VERSION << 3); >> - p->next_action_time = G.cur_time; /* = set_next(p, 0); */ >> reset_peer_stats(p, STEP_THRESHOLD); >> >> llist_add_to(&G.ntp_peers, p); >> @@ -2263,8 +2255,8 @@ static NOINLINE void ntp_init(char **argv) >> if (opts & OPT_N) >> setpriority(PRIO_PROCESS, 0, -15); >> >> - /* add_peers() calls can retry DNS resolution (possibly forever). >> - * Daemonize before them, or else boot can stall forever. >> + /* add_peers() calls can retry DNS resolution. >> + * Daemonize before them, or else boot can stall. >> */ >> if (!(opts & OPT_n)) { >> bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO, argv); >> @@ -2378,7 +2370,10 @@ int ntpd_main(int argc UNUSED_PARAM, char **argv) >> for (item = G.ntp_peers; item != NULL; item = item->link) { >> peer_t *p = (peer_t *) item->data; >> >> - if (p->next_action_time <= G.cur_time) { >> + if (!p->p_lsa) { >> + resolve_peer_hostname(p); >> + >> + } else if (p->next_action_time <= G.cur_time) { >> if (p->p_fd == -1) { >> /* Time to send new req */ >> if (--cnt == 0) { >> @@ -2400,7 +2395,7 @@ int ntpd_main(int argc UNUSED_PARAM, char **argv) >> >> /* What if don't see it because it >> changed its IP? */ >> if (p->reachable_bits == 0) >> - resolve_peer_hostname(p, >> /*loop_on_fail=*/ 0); >> + resolve_peer_hostname(p); >> >> set_next(p, timeout); >> } > _______________________________________________ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox