libc/resolv: Refactor the configuration parser
AcceptedPublic
Actions

Authored by des on Sun, Jun 28, 8:13 AM.

Details

Reviewers

kevans
markj

Summary

This was previously all a single loop in res_init(), apart from option
parsing which we cleaned up in a previous commit. Break it out into
separate functions for reading the configuration line by line, setting
the default domain, setting the search list, and adding a nameserver
to the nameserver list. Sprinkle bounds checks and code comments all
around.

The sortlist code, which has been disabled for the past 20 years, will
be dealt with in a separate commit.

MFC after: 1 week

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 74356
Build 71239: arc lint + arc unit

Event Timeline

des created this revision.Sun, Jun 28, 8:13 AM

Herald added a subscriber: imp. · View Herald TranscriptSun, Jun 28, 8:13 AM

des requested review of this revision.Sun, Jun 28, 8:13 AM

des added a parent revision: D57923: libc/resolv: Refactor the option parser.

des added a child revision: D57925: libc/resolv: Reimplement the sortlist parser.

Harbormaster completed remote builds in B74351: Diff 180853.Sun, Jun 28, 8:14 AM

tweak

Harbormaster completed remote builds in B74356: Diff 180858.Sun, Jun 28, 8:47 AM

My comments are just nitpicking.

lib/libc/resolv/res_init.c
250	What is this comment format?
333	After reading res_setoptions(), I kind of prefer using chained `else if`s rather than `if (...) { ... continue; }` for each directive.
486	I think it's not possible to have end == NULL? And the caller has set `*eol = '\0'` so I wonder if it's cleaner to just not pass `end` to these helpers at all.
581	It'd help to describe the return value. And why not use a bool?
600	Here we also can't have `end == NULL`.
623	Might as well print the error string too...?

This revision is now accepted and ready to land.Tue, Jun 30, 2:36 PM

des added inline comments.Tue, Jun 30, 4:21 PM

lib/libc/resolv/res_init.c
250	ISC's in-house style
333	me too, but it makes it harder to cleanly `#ifdef` out individual stanzas
486	You're right, `res_ninit()` originally passed `NULL` here but no longer does. The point of passing `end` around is that if I already have it it's cheaper to pass it than to recompute it.
581	It's technically a count...
600	Correct, there was an earlier version of the patch were we could
623	It's not very useful, we're using `AI_NUMERICHOST` so the only possible error is a syntax error which I think will show as “Name does not resolve”.

Revision Contents
Changeset List

Path

Size

lib/

libc/

resolv/

res_init.c

669 lines

Diff 180858

View Options

lib/libc/resolv/res_init.c

libc/resolv: Refactor the configuration parserAcceptedPublicActions