Hello Robin,

On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens@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.