Add csp_iflist_remove function

[sigmundklaa]

Implements a csp_iflist_remove function to go along with csp_iflist_add. This is useful to ensure full cleanup in case initialization of an interface fails for whatever reason.

[yashi]

I know it's frustrating not to fix white spaces, but let's keep focused on what the commit is intended to do. You can add white space fixes if you really want as a separate commit.

[yashi]

You don't need to scan the rest of the list. break it. You can also if->next = NULL here so that when failed to find the target in the list, we don't need to touch the ifc.

[sigmundklaa]

Yes will do. However is it better to not touch ifc when it is not found in the list? If for some reason an interface not in the list is passed in as argument, is it not better to ensure that it does not contain any reference to other elements in the list, so that any call to csp_iflist_remove guarantees ifc will be completely seperate from the list?

[yashi]

Well, IMO, if you don't find the given interface in the list, you are pretty much screwed up already. The csp_iflist API doesn't exclusively maintain all interfaces added. Once you add an interface to the list, all interfaces are linked. If you mess up one of the linked interfaces you get from, say, csp_iflist_get_by_subnet(), the whole list gets messed up.

So if you can't find the given interface, that means you passed:

1. an already-removed interface,
2. a completely wrong pointer, or
3. you messed up the memory area.

If you passed an already removed interface, ifc->next should already be NULL.

If you passed a wrong pointer, assigning NULL to ifc->next would mean destroying some memory area.

If you have already messed up your memory, you are already done.

Therefore, I don't see the point of setting ifc->next to NULL. What am I missing?

[sigmundklaa]

Ok thats fair, I'll fix it

[yashi]

LGTM. Please do me a last favor. Squash the three commits in one, and copy paste your first comment in this thread in the commit message. If you allow me, I can do that for you.

[yashi]

(just in case, please do not squash any other PRs! The squash request on this PR was that because the three commits are logically one change. It may or may not apply to others. Thanks)

[yashi]

As I said in the other place, it LGTM.

We should merge three commits in this PR in one commit because it's logically one thing. If you allow me to push to your branch, I can fix it, and let you check before merge. Otherwise, I can just do that locally and merge to the develop branch.

[sigmundklaa]

As I said in the other place, it LGTM.

We should merge three commits in this PR in one commit because it's logically one thing. If you allow me to push to your branch, I can fix it, and let you check before merge. Otherwise, I can just do that locally and merge to the develop branch.

Yes sorry, I forgot about this. I have squashed the last three now

[yashi]

@sigmundklaa Thank you for squashing!

I know I said "last" but would you please fold the commit message, like this?
git doesn't like long lines.

Implements a csp_iflist_remove function to go along with
csp_iflist_add. This is useful to ensure full cleanup in case
initialization of an interface fails for whatever reason.

[sigmundklaa]

@sigmundklaa Thank you for squashing!

I know I said "last" but would you please fold the commit message, like this? git doesn't like long lines.

Implements a csp_iflist_remove function to go along with
csp_iflist_add. This is useful to ensure full cleanup in case
initialization of an interface fails for whatever reason.

Done