Re: [PATCH v3 0/5] Fix Bug#12935 + cosmetic changes/enhancements

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 2863 bytes --]

Hello Robin,

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> Hi all
> 
> After carefully reviewing and adopting all comments Michael had about
> the code in previous patchset, here is again a new version of it.
> 
> Only PATCH 1 was changed since v2:
> - propper initialization of variables where required preventing possible
>  undefined behaviour
> - more reliable error checking: all functions now return a meaningfull
>  integer one way or another to indicate if and what kind of error
>  happend. No need anymore to set errno to 0 manually as it no longer
>  depended on.
> - No more checking against NULL making many comparisions easier for the
>  eye
> - fix some possible memory leaks
> - fix a possible unallocation of not yet allocated memory
> - in case of a system error, a descriptive error is shown instead of
>  the number, using the %m directive.
> 
> Hoping to pass the required strict evaluation this time :-).

Yes, I think so.

It is a huge patchset, so hard to review, but I am happy with it and happy to put my stamp on it :)

Thank you very much for your very hard work on this and all those hours that you put in.

What is coming next? :)

-Michael

> For reference, the content of the summary mail that was sent with v1 of
> the patch:
> ---
> This patchset fixes Bug#12935
> (https://bugzilla.ipfire.org/show_bug.cgi?id=12935)
> 
> Summary:
> Addons where the initscript does not match the addon-name and addons with
> multiple initscripts are now listed on services.cgi since CU170.
> But addonctrl still expected addon name to be equal to
> initscript name; Hence starting/stopping/enabling/disabling of such
> addons was not possible.
> This has always been like that, but that problem was hidden as
> services.cgi also did not display those addon services.
> 
> After discussing this with Adolf on the Bug report, we concluded that we
> should adapt addonctrl to work with the new addon metadata
> Services-field instead.
> 
> I basically rewrote addonctrl to not only use the new services metadata
> but also to have better errorchecking and added the posibility to check
> if a service is currently enabled or disabled.
> As a result services.cgi no longer has to go checking the precense of
> runlevel initscripts, but can just ask addonctrl.
> I also added a warning to services.cgi if a runlevel initscript does not
> exists, to prevent the user from wondering why he can't enable a
> specific service. (Adolf pointed out some services don't install
> runlevel initscripts by default)
> 
> More details in the bugreport and in the commit-messages of the patches.
> 
> Regards
> Robin
> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>