Re: [RFC] Automated generation of /etc/resolv.conf from the rc.d script

Jeremie, good day.

Wed, Apr 23, 2008 at 09:06:59PM +0200, Jeremie Le Hen wrote:
> On Mon, Apr 14, 2008 at 07:44:13PM +0400, Eygene Ryabinkin wrote:
> > [...]
> > Testing and feedback are more than welcome.
> 
> I didn't test your patch but I have a have a few comments about it:
> 
> In install_new_file(), I don't think you should test for $CMP being an
> executable file...  It is in the base system and the rule of thumb in
> other rc.d scripts is to use those directly.

OK, I just followed the practice of dhclient-script, since some of
the code was written after looking at that file.  But if the
FreeBSD'sh way to do it is just to use $CMP as is, I am not against
it.  Although, such check should not harm anything and since it
uses built-in '[' command, it even does not invoke fork/exec sequence.
But it uses fstat ;))  I had not changed it yet, but I am currently
thinking about it.  More opinions are welcome ;))

> I'm not sure you should chown/chmod the forwarders file.  People may
> have custom setup that you should not interfere with without a good
> reason.

OK, I will do chown/chmod only for previously nonexistent forwarders
file.  No changes will be done for the existing object.

> Also, I would rather let add_new_bind_forwarders() build the "empty"
> forwarders file, it would make more sense IMHO.  You could then put a
> single call to add_new_bind_forwarders() at the end of the script under
> a $resolv_build_named_forwarders condition.  It makes more sense indeed
> to test this outside of the function, my personal feeling being that it
> makes the reading less puzzling.

OK, I took your idea and developed it a bit further: now the if-elif-else
branch just sets some variables and the creation of the files tooks
place at the end of the script.

The patch with these modifications is attached.  It is not very much
tested by me now, so I will not add it to the PR until it will receive
the good amount of testing.  But I am attaching it here for you and
others to have the possibility to test and/or comment.

I had slightly changed the second patch, making temporary filenames
be PID-based to catch two concurrent instances of the script running
at the same time.  This is not the best way to do it, but at least
file contents will be sane.  The modified version of the second patch
is also attached.  It needs testing too, so it is preliminary as well.

> Anyway, thank you very much for your work.  I think many people will
> enjoy it once it will hit the source tree.

Thank you!
-- 
Eygene