Re: replace finalize_files() ?

[Jan-Henrik Haukeland]

Martin Pala <address@hidden> writes:

> Hi,
>
> i think it could be good to replace finalize_files() function with
> direct call of unlink(Run.pidfile). The present implementation does
> exactly the same thing:
>
> --SNIP--
> /**
>  * Remove temporary files
>  */
> void finalize_files() {
>
>   unlink(Run.pidfile);
>
> }
> --SNIP--
>
> In the code are on the same level as finalize_files() called other
> standalone file handling functions for similar intent - such as
> state_finalize(), log_close(), etc. I think there is no reason to keep
> it - the code can be more clear without it and the code can be faster
> without it (it will not improve performance very much, but why to pay
> for something, which is not necesary :)
>
> May we replace it?

Ah, if you feel strongly for it then replace it (although I have to
say that I like it). The intention is to have a common placeholder for
finalizing all files used within monit, but since state does it's own
unlinking[*] only the monit.pidfile is finalized in this function, but
this may change in the future(?).

The reason for having state_finalize() is based on the same idea, even
if it now only unlinks the state.file, but other state cleanup code
may be necessary in the future, so it's also a placeholder. But to
keep the rationale for having finalize_files, maybe instead of
removing it, we should put the unlink of the state file in this
function and remove state_finalize() instead. It's better, at least in
my book.

* BTW, Instead of using unlink(2) maybe we should replace unlink with
  remove(3) since this is the ANSI version.

PS. The argument for execution speed is a long shoot though. The time
for *not* calling a non-system and a non-thirdpart library function
will probably save time in the nano-scale only ;-)

-- 
Jan-Henrik Haukeland