On 7/11/17, Lucas Gabriel Vuotto <[email protected]> wrote:
> I like the idea of this patch. There're two little details marked below.
>
>> @@ -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.
>> @@ -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.
