Re: [hackers] [scron] [PATCH 3/3] Keep existing crontab entries if loading new entries fails

From: Michael Forney <mforney_AT_mforney.org>
Date: Tue, 11 Jul 2017 10:54:47 -0700

On 7/11/17, Lucas Gabriel Vuotto <lvuotto92_AT_gmail.com> wrote:
> I like the idea of this patch. There're two little details marked below.
>
>> _AT_@ -391,7 +379,7 @@ freecte(struct ctabentry *cte, int nfields)
>> }
>>
>> static void
>> -unloadentries(void)
>> +unloadentries(struct ctabentry *ctab)
>
> Shadowing variables is error prone. In this case, this will happen if the
> first patch is merged, to which I oppose, given that there is nothing wrong
> with queue.h macros.

Sure, I can rename it. But anyway, we need unloadentries to take a
parameter regardless of whether queue.h is used or not since it either
needs to unload a partially loaded ctab, or the old ctab.

>> _AT_@ -492,14 +483,6 @@ loadentries(void)
>> }
>>
>> static void
>> -reloadentries(void)
>> -{
>> - unloadentries();
>> - if (loadentries() < 0)
>> - logwarn("warning: discarding old crontab entries\n");
>> -}
>> -
>> -static void> sighandler(int sig)
>> {
>> switch (sig) {
>
> Somehow, the sysadmin should be informed that loadentries failed. Either
> keep reloadentries or log the failure in main.

They will be informed about being out of memory or about the
particular parse error that failed. But I suppose it's worth calling
out specifically that the new version of the file wasn't loaded.
Received on Tue Jul 11 2017 - 19:54:47 CEST