The following reply was made to PR conf/165817; it has been noted by GNATS.
From: John Baldwin <[email protected]> To: Volodymyr Kostyrko <[email protected]> Cc: [email protected] Subject: Re: conf/165817: [periodic] [patch] /etc/periodic reports misconfiguration when it shouldn't Date: Tue, 08 May 2012 10:52:37 -0400 On 5/8/12 3:57 AM, Volodymyr Kostyrko wrote: > John Baldwin wrote: >> This doesn't make sense. The various variables don't have a default >> value in >> /etc/defaults/rc.conf (e.g. daily_local), so they should just be >> empty, and >> the for loop should not do anything if the variable is empty. For >> example, if >> you run this in /bin/sh: >> >> $ for script in $notexists >>> do >>> echo foo >>> done >> $ >> >> You don't get any output at all. Thus, having the default >> configuration of >> not having these variables set should never get into the loop to >> execute the >> line you are modifying. >> >> In your case you must have daily_local, etc. set to some absolute path >> that >> doesn't exist (e.g. daily_local="/nonexistent") in which case that is a >> misconfiguration that the scripts should warn you about. >> >> Or is the problem that you have daily_local set to "/etc/*.local" (the >> glob) >> and that isn't matching, so the shell runs the loop with the value >> "/etc/*.local"? That is a bit harder to fix. >> >> Your patch would not be correct if someone set "daily_local" to >> "/nonexistent". That is a case that _should_ be warned about. > > Yes, I should probably rethink the patch. It definitely should warn > about /nonexistent. > > That makes me somehow stuck. periodic scripts source > /etc/defaults/periodic.conf which defines this variables as > "/etc/*.local". So when it comes to checking whether this file is > available the variable is already set. The possible solution is to check > whether variable is set to the default path and skip warning if default > path is unexistent or adding empty placeholders for required files. I > personally prefer the latter to keep scripts as simple as possible and > strip all signs of artificial intelligence aka weird logic. Oof, now I see. I was looking at /etc/defaults/rc.conf rather than /etc/defaults/periodic.conf for some reason. We certainly should not warn in the default install. I'll think about this some more. -- John Baldwin _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "[email protected]"
