[Buildroot] [PATCH v1] package/mstpd: fix mstpd bridge-stp use of pidof

[Buildroot] [PATCH v1] package/mstpd: fix mstpd bridge-stp use of pidof

[Colin Foster]

Through mstpd version 0.1.0, the bridge-stp script uses the '-c'
option to the pidof command. Busybox does not support this option, so
mstpd does not work.

This has been fixed in the main development branch of mstpd, but it is
unclear when the next release will be. In the meantime, apply the fix
here so that mstpd will be useable until the next version release.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 ...pport-different-versions-of-pidof-13.patch | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch

diff --git a/package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch b/package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch
new file mode 100644
index 0000000000..daa591131b
--- /dev/null
+++ b/package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch
@@ -0,0 +1,48 @@
+From 181c453fc1a00573e19f14960dcc54ad84beea7c Mon Sep 17 00:00:00 2001
+From: colin-foster-in-advantage <colin.foster@in-advantage.com>
+Date: Tue, 12 Jul 2022 23:01:09 -0700
+Subject: [PATCH] bridge-stp.in: support different versions of pidof (#137)
+
+* bridge-stp.in: support different versions of pidof
+
+Busybox uses a version of pdiof that doesn't support the -c option. As
+such, this renders mstpd non-functional on any Busybox system.
+
+Just use the standard form of pidof to detect any running instances of mstpd.
+
+Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
+---
+ bridge-stp.in | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/bridge-stp.in b/bridge-stp.in
+index 47cbe79..3807873 100755
+--- a/bridge-stp.in
++++ b/bridge-stp.in
+@@ -139,7 +139,7 @@ case "$action" in
+         fi
+ 
+         # Start mstpd if necessary.
+-        if ! pidof -c -s mstpd >/dev/null; then
++        if ! pidof -s mstpd >/dev/null; then
+             if [ "$MANAGE_MSTPD" != 'y' ]; then
+                 errmsg 'mstpd is not running'
+                 exit 3
+@@ -212,12 +212,12 @@ case "$action" in
+         done
+ 
+         # Kill mstpd, since no bridges are currently using it.
+-        kill $(pidof -c mstpd)
++        kill $(pidof mstpd)
+         ;;
+     restart|restart_config)
+         if [ "$action" = 'restart' ]; then
+             # Kill mstpd.
+-            pids="$(pidof -c mstpd)" ; Err=$?
++            pids="$(pidof mstpd)" ; Err=$?
+             if [ $Err -eq 0 ]; then
+                 echo 'Stopping mstpd ...'
+                 kill $pids
+-- 
+2.25.1
+
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1] package/mstpd: fix mstpd bridge-stp use of pidof

[Arnout Vandecappelle]

On 20/07/2022 19:11, Colin Foster wrote:
> Through mstpd version 0.1.0, the bridge-stp script uses the '-c'
> option to the pidof command. Busybox does not support this option, so
> mstpd does not work.
> 
> This has been fixed in the main development branch of mstpd, but it is
> unclear when the next release will be. In the meantime, apply the fix
> here so that mstpd will be useable until the next version release.
> 
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>

  Applied to master, thanks.

  Would it be useful to add a runtime test for mstpd to detect such issues?

  Regards,
  Arnout

> ---
>   ...pport-different-versions-of-pidof-13.patch | 48 +++++++++++++++++++
>   1 file changed, 48 insertions(+)
>   create mode 100644 package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch
> 
> diff --git a/package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch b/package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch
> new file mode 100644
> index 0000000000..daa591131b
> --- /dev/null
> +++ b/package/mstpd/0001-bridge-stp.in-support-different-versions-of-pidof-13.patch
> @@ -0,0 +1,48 @@
> +From 181c453fc1a00573e19f14960dcc54ad84beea7c Mon Sep 17 00:00:00 2001
> +From: colin-foster-in-advantage <colin.foster@in-advantage.com>
> +Date: Tue, 12 Jul 2022 23:01:09 -0700
> +Subject: [PATCH] bridge-stp.in: support different versions of pidof (#137)
> +
> +* bridge-stp.in: support different versions of pidof
> +
> +Busybox uses a version of pdiof that doesn't support the -c option. As
> +such, this renders mstpd non-functional on any Busybox system.
> +
> +Just use the standard form of pidof to detect any running instances of mstpd.
> +
> +Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> +---
> + bridge-stp.in | 6 +++---
> + 1 file changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/bridge-stp.in b/bridge-stp.in
> +index 47cbe79..3807873 100755
> +--- a/bridge-stp.in
> ++++ b/bridge-stp.in
> +@@ -139,7 +139,7 @@ case "$action" in
> +         fi
> +
> +         # Start mstpd if necessary.
> +-        if ! pidof -c -s mstpd >/dev/null; then
> ++        if ! pidof -s mstpd >/dev/null; then
> +             if [ "$MANAGE_MSTPD" != 'y' ]; then
> +                 errmsg 'mstpd is not running'
> +                 exit 3
> +@@ -212,12 +212,12 @@ case "$action" in
> +         done
> +
> +         # Kill mstpd, since no bridges are currently using it.
> +-        kill $(pidof -c mstpd)
> ++        kill $(pidof mstpd)
> +         ;;
> +     restart|restart_config)
> +         if [ "$action" = 'restart' ]; then
> +             # Kill mstpd.
> +-            pids="$(pidof -c mstpd)" ; Err=$?
> ++            pids="$(pidof mstpd)" ; Err=$?
> +             if [ $Err -eq 0 ]; then
> +                 echo 'Stopping mstpd ...'
> +                 kill $pids
> +--
> +2.25.1
> +
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1] package/mstpd: fix mstpd bridge-stp use of pidof

[Colin Foster]

On Sat, Jul 23, 2022 at 02:47:51PM +0200, Arnout Vandecappelle wrote:
> 
> 
> On 20/07/2022 19:11, Colin Foster wrote:
> > Through mstpd version 0.1.0, the bridge-stp script uses the '-c'
> > option to the pidof command. Busybox does not support this option, so
> > mstpd does not work.
> > 
> > This has been fixed in the main development branch of mstpd, but it is
> > unclear when the next release will be. In the meantime, apply the fix
> > here so that mstpd will be useable until the next version release.
> > 
> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> 
>  Applied to master, thanks.
> 
>  Would it be useful to add a runtime test for mstpd to detect such issues?

That was my first fix attempt - run `pidof -c init` and see if it
succeeded. Fall back to just `pidof` otherwise.

We decided to just drop the -c argument entirely, since it probably
wasn't needed.

> 
>  Regards,
>  Arnout
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v1] package/mstpd: fix mstpd bridge-stp use of pidof

[Peter Korsgaard]

>>>>> "Colin" == Colin Foster <colin.foster@in-advantage.com> writes:

 > Through mstpd version 0.1.0, the bridge-stp script uses the '-c'
 > option to the pidof command. Busybox does not support this option, so
 > mstpd does not work.

 > This has been fixed in the main development branch of mstpd, but it is
 > unclear when the next release will be. In the meantime, apply the fix
 > here so that mstpd will be useable until the next version release.

 > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>

Committed to 2022.05.x and 2022.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot