[Buildroot] [PATCH 0/3] package/ssdp-responder bump and check-package fixes

[Buildroot] [PATCH 0/3] package/ssdp-responder bump and check-package fixes

[Joachim Wiberg]

Hi,

here's the upgrade of the ssdpd-responder package to the latest release,
as of yesterday.  Also included are some stilistic fixes to Config.in
and the start script.  The latter changes suggested by check-package and
shellcheck, fixes based in part on the SMCRoute start script.

Best regards
 /Joachim

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

[Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Joachim Wiberg]

Changes:
 - New command line options to ssdp-scan
 - Update copyright years (affects LICENSE file hash)

Fixes:
 - Workaround for OpenVPN /32 default server setup
 - Time-of-check vs time-of-use issue with caching of UUID,
   found by Coverity Scan, fixed by Raul Porancea

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 package/ssdp-responder/ssdp-responder.hash | 4 ++--
 package/ssdp-responder/ssdp-responder.mk   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/ssdp-responder/ssdp-responder.hash b/package/ssdp-responder/ssdp-responder.hash
index e8cbbb4a1f..425ad86e08 100644
--- a/package/ssdp-responder/ssdp-responder.hash
+++ b/package/ssdp-responder/ssdp-responder.hash
@@ -1,3 +1,3 @@
 # Locally calculated
-sha256  7ae49229e7c7a55fed9e36598b12e2173eecef0fffe0a386b6a10fad30f3c79f  ssdp-responder-1.8.tar.gz
-sha256  e17dc0bc91bf499d8cca5e016c22c6d2a4770e3cc1a43756a7973375a83ddb90  LICENSE
+sha256  974c244abd4ba8c87532867a84756182a1460c99072ffb1eb91c5a1f73311d89  ssdp-responder-1.9.tar.gz
+sha256  68d6fdc22e337f725fe719bf9ae6d92b1d8d0ca4cff8219b303ab76706670a8d  LICENSE
diff --git a/package/ssdp-responder/ssdp-responder.mk b/package/ssdp-responder/ssdp-responder.mk
index 3fee4c2006..fd7b1ea44b 100644
--- a/package/ssdp-responder/ssdp-responder.mk
+++ b/package/ssdp-responder/ssdp-responder.mk
@@ -4,7 +4,7 @@
 #
 ################################################################################
 
-SSDP_RESPONDER_VERSION = 1.8
+SSDP_RESPONDER_VERSION = 1.9
 SSDP_RESPONDER_SITE = https://github.com/troglobit/ssdp-responder/releases/download/v$(SSDP_RESPONDER_VERSION)
 SSDP_RESPONDER_LICENSE = ISC
 SSDP_RESPONDER_LICENSE_FILES = LICENSE
-- 
2.34.1

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

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Thomas Petazzoni via buildroot]

On Mon, 31 Oct 2022 18:46:30 +0100
Joachim Wiberg <troglobit@gmail.com> wrote:

> Changes:
>  - New command line options to ssdp-scan
>  - Update copyright years (affects LICENSE file hash)
> 
> Fixes:
>  - Workaround for OpenVPN /32 default server setup
>  - Time-of-check vs time-of-use issue with caching of UUID,
>    found by Coverity Scan, fixed by Raul Porancea
> 
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Thanks for the patch! One request below.

> ---
>  package/ssdp-responder/ssdp-responder.hash | 4 ++--
>  package/ssdp-responder/ssdp-responder.mk   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/package/ssdp-responder/ssdp-responder.hash b/package/ssdp-responder/ssdp-responder.hash
> index e8cbbb4a1f..425ad86e08 100644
> --- a/package/ssdp-responder/ssdp-responder.hash
> +++ b/package/ssdp-responder/ssdp-responder.hash
> @@ -1,3 +1,3 @@
>  # Locally calculated
> -sha256  7ae49229e7c7a55fed9e36598b12e2173eecef0fffe0a386b6a10fad30f3c79f  ssdp-responder-1.8.tar.gz
> -sha256  e17dc0bc91bf499d8cca5e016c22c6d2a4770e3cc1a43756a7973375a83ddb90  LICENSE
> +sha256  974c244abd4ba8c87532867a84756182a1460c99072ffb1eb91c5a1f73311d89  ssdp-responder-1.9.tar.gz
> +sha256  68d6fdc22e337f725fe719bf9ae6d92b1d8d0ca4cff8219b303ab76706670a8d  LICENSE

The hash of the license file is changed, this should be explained in
the commit log (what are the changes, do they require changing the
license information in the .mk file).

Silently changing the license file hashes defeats the entire purpose of
having hashes for license files, which is precisely to carefully verify
what has changed in the license file.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Joachim Wiberg]

Hi Thomas!

On Mon, Oct 31, 2022 at 21:47, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> On Mon, 31 Oct 2022 18:46:30 +0100
> Joachim Wiberg <troglobit@gmail.com> wrote:
>> Changes:
>>  - Update copyright years (affects LICENSE file hash)
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> -sha256  e17dc0bc91bf499d8cca5e016c22c6d2a4770e3cc1a43756a7973375a83ddb90  LICENSE
>> +sha256  68d6fdc22e337f725fe719bf9ae6d92b1d8d0ca4cff8219b303ab76706670a8d  LICENSE
> The hash of the license file is changed, this should be explained in
> the commit log (what are the changes, do they require changing the
> license information in the .mk file).

I tried to be as clear as possible about this, even when I did the
release upstream.  Maybe I should have been extra clear somehow?

> Silently changing the license file hashes defeats the entire purpose of
> having hashes for license files, which is precisely to carefully verify
> what has changed in the license file.

Yes, I fully understand.


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

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Thomas Petazzoni via buildroot]

On Tue, 01 Nov 2022 08:18:53 +0100
Joachim Wiberg <troglobit@gmail.com> wrote:

> I tried to be as clear as possible about this, even when I did the
> release upstream.  Maybe I should have been extra clear somehow?

All we expect is a short note in the commit log, which clarifies that
the submitter of the patch has changed the hash of the license file
after properly checking what the changes in the license files are.
That's really the complete extent of our expectation :-)

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Yann E. MORIN]

Thomas, All,

On 2022-11-02 18:06 +0100, Thomas Petazzoni via buildroot spake thusly:
> On Tue, 01 Nov 2022 08:18:53 +0100
> Joachim Wiberg <troglobit@gmail.com> wrote:
> > I tried to be as clear as possible about this, even when I did the
> > release upstream.  Maybe I should have been extra clear somehow?
> All we expect is a short note in the commit log, which clarifies that
> the submitter of the patch has changed the hash of the license file
> after properly checking what the changes in the license files are.
> That's really the complete extent of our expectation :-)

I think you missed the part where Joachim *does* priovide that
information  in his commit log, copied below:

    Changes:
     - New command line options to ssdp-scan
     - Update copyright years (affects LICENSE file hash)

So, our expectations were met! ;-)

Acked-by: Yann E. MORIN <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Thomas Petazzoni via buildroot]

On Wed, 2 Nov 2022 22:19:23 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> I think you missed the part where Joachim *does* priovide that
> information  in his commit log, copied below:
> 
>     Changes:
>      - New command line options to ssdp-scan
>      - Update copyright years (affects LICENSE file hash)
> 
> So, our expectations were met! ;-)

Sorry about that, I definitely read too quickly, my bad :/

My apologizes Joachim for missing this: it was all good from the start!

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Joachim Wiberg]

On Wed, 2022-11-02 at 22:21 +0100, Thomas Petazzoni wrote:
> On Wed, 2 Nov 2022 22:19:23 +0100
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > I think you missed the part where Joachim *does* priovide that
> > information  in his commit log, copied below:
> >     Changes:
> >      - New command line options to ssdp-scan
> >      - Update copyright years (affects LICENSE file hash)
> > So, our expectations were met! ;-)

Thanks Yann! <3

> Sorry about that, I definitely read too quickly, my bad :/
> My apologizes Joachim for missing this: it was all good from the
> start!

No worries, I know how stressful things can get.  Was just
so confusing from my end :-D

Take care!
 /J

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

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Yann E. MORIN]

Joachime, All,

On 2022-10-31 18:46 +0100, Joachim Wiberg spake thusly:
> Changes:
>  - New command line options to ssdp-scan
>  - Update copyright years (affects LICENSE file hash)
> 
> Fixes:
>  - Workaround for OpenVPN /32 default server setup
>  - Time-of-check vs time-of-use issue with caching of UUID,
>    found by Coverity Scan, fixed by Raul Porancea
> 
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/ssdp-responder/ssdp-responder.hash | 4 ++--
>  package/ssdp-responder/ssdp-responder.mk   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/package/ssdp-responder/ssdp-responder.hash b/package/ssdp-responder/ssdp-responder.hash
> index e8cbbb4a1f..425ad86e08 100644
> --- a/package/ssdp-responder/ssdp-responder.hash
> +++ b/package/ssdp-responder/ssdp-responder.hash
> @@ -1,3 +1,3 @@
>  # Locally calculated
> -sha256  7ae49229e7c7a55fed9e36598b12e2173eecef0fffe0a386b6a10fad30f3c79f  ssdp-responder-1.8.tar.gz
> -sha256  e17dc0bc91bf499d8cca5e016c22c6d2a4770e3cc1a43756a7973375a83ddb90  LICENSE
> +sha256  974c244abd4ba8c87532867a84756182a1460c99072ffb1eb91c5a1f73311d89  ssdp-responder-1.9.tar.gz
> +sha256  68d6fdc22e337f725fe719bf9ae6d92b1d8d0ca4cff8219b303ab76706670a8d  LICENSE
> diff --git a/package/ssdp-responder/ssdp-responder.mk b/package/ssdp-responder/ssdp-responder.mk
> index 3fee4c2006..fd7b1ea44b 100644
> --- a/package/ssdp-responder/ssdp-responder.mk
> +++ b/package/ssdp-responder/ssdp-responder.mk
> @@ -4,7 +4,7 @@
>  #
>  ################################################################################
>  
> -SSDP_RESPONDER_VERSION = 1.8
> +SSDP_RESPONDER_VERSION = 1.9
>  SSDP_RESPONDER_SITE = https://github.com/troglobit/ssdp-responder/releases/download/v$(SSDP_RESPONDER_VERSION)
>  SSDP_RESPONDER_LICENSE = ISC
>  SSDP_RESPONDER_LICENSE_FILES = LICENSE
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Yann E. MORIN]

Joachim, All,

On 2022-11-05 19:45 +0100, Yann E. MORIN spake thusly:
> On 2022-10-31 18:46 +0100, Joachim Wiberg spake thusly:
> > Changes:
> >  - New command line options to ssdp-scan
> >  - Update copyright years (affects LICENSE file hash)
> > 
> > Fixes:
> >  - Workaround for OpenVPN /32 default server setup
> >  - Time-of-check vs time-of-use issue with caching of UUID,
> >    found by Coverity Scan, fixed by Raul Porancea
> > 
> > Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
> 
> Applied to master, thanks.

s/master/next/

Also, sorry for the typo in your name in my previous reply... :-/

Regards,
Yann E. MORIN.

> Regards,
> Yann E. MORIN.
> 
> > ---
> >  package/ssdp-responder/ssdp-responder.hash | 4 ++--
> >  package/ssdp-responder/ssdp-responder.mk   | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/package/ssdp-responder/ssdp-responder.hash b/package/ssdp-responder/ssdp-responder.hash
> > index e8cbbb4a1f..425ad86e08 100644
> > --- a/package/ssdp-responder/ssdp-responder.hash
> > +++ b/package/ssdp-responder/ssdp-responder.hash
> > @@ -1,3 +1,3 @@
> >  # Locally calculated
> > -sha256  7ae49229e7c7a55fed9e36598b12e2173eecef0fffe0a386b6a10fad30f3c79f  ssdp-responder-1.8.tar.gz
> > -sha256  e17dc0bc91bf499d8cca5e016c22c6d2a4770e3cc1a43756a7973375a83ddb90  LICENSE
> > +sha256  974c244abd4ba8c87532867a84756182a1460c99072ffb1eb91c5a1f73311d89  ssdp-responder-1.9.tar.gz
> > +sha256  68d6fdc22e337f725fe719bf9ae6d92b1d8d0ca4cff8219b303ab76706670a8d  LICENSE
> > diff --git a/package/ssdp-responder/ssdp-responder.mk b/package/ssdp-responder/ssdp-responder.mk
> > index 3fee4c2006..fd7b1ea44b 100644
> > --- a/package/ssdp-responder/ssdp-responder.mk
> > +++ b/package/ssdp-responder/ssdp-responder.mk
> > @@ -4,7 +4,7 @@
> >  #
> >  ################################################################################
> >  
> > -SSDP_RESPONDER_VERSION = 1.8
> > +SSDP_RESPONDER_VERSION = 1.9
> >  SSDP_RESPONDER_SITE = https://github.com/troglobit/ssdp-responder/releases/download/v$(SSDP_RESPONDER_VERSION)
> >  SSDP_RESPONDER_LICENSE = ISC
> >  SSDP_RESPONDER_LICENSE_FILES = LICENSE
> > -- 
> > 2.34.1
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3] package/ssdp-responder: bump to version 1.9

[Joachim Wiberg]

On Sat, Nov 05, 2022 at 19:48, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> On 2022-11-05 19:45 +0100, Yann E. MORIN spake thusly:
>> Applied to master, thanks.
> s/master/next/
> Also, sorry for the typo in your name in my previous reply... :-/

Absolutely no problem, thank you! <3

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

[Buildroot] [PATCH 2/3] package/ssdp-responder: minor update of help text

[Joachim Wiberg]

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 package/ssdp-responder/Config.in | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/package/ssdp-responder/Config.in b/package/ssdp-responder/Config.in
index df57546eed..8b5dfe67a6 100644
--- a/package/ssdp-responder/Config.in
+++ b/package/ssdp-responder/Config.in
@@ -7,14 +7,16 @@ config BR2_PACKAGE_SSDP_RESPONDER
 	  targeted more at embedded systems that need to announce
 	  themselves to Windows systems.
 
-	  ssdpd is a stand-alone UNIX, no external dependencies but the
-	  standard C library.  It has a built-in web server for serving
-	  the UPnP XML description which Windows use to present the
-	  icon, by default an InternetGatewayDevice is announced.
+	  The project consists of ssdpd and ssdp-scan.  The former is a
+	  stand-alone UNIX daemon, it has no dependencies but a standard
+	  C library.  It comes with built-in web server for serving the
+	  UPnP XML description on port 1901, the XML is used by Windows
+	  to present the icon, by default an InternetGatewayDevice is
+	  announced.
 
-	  Also included is ssdp-scan, a tool similar to mdns-scan, which
-	  continuously scans for SSDP capable hosts on the network.
-	  Take care only to use for debugging since it scans the network
-	  quite aggressively.
+	  ssdp-scan is a tool, similar to mdns-scan, to find other SSDP
+	  capable hosts on the network.  Take care to only use it for
+	  debugging, or quicker device discovery operations, since it
+	  scans the network quite aggressively.
 
 	  https://github.com/troglobit/ssdp-responder/
-- 
2.34.1

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

Re: [Buildroot] [PATCH 2/3] package/ssdp-responder: minor update of help text

[Yann E. MORIN]

Joachim, All,

On 2022-10-31 18:46 +0100, Joachim Wiberg spake thusly:
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Applied to next, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/ssdp-responder/Config.in | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/package/ssdp-responder/Config.in b/package/ssdp-responder/Config.in
> index df57546eed..8b5dfe67a6 100644
> --- a/package/ssdp-responder/Config.in
> +++ b/package/ssdp-responder/Config.in
> @@ -7,14 +7,16 @@ config BR2_PACKAGE_SSDP_RESPONDER
>  	  targeted more at embedded systems that need to announce
>  	  themselves to Windows systems.
>  
> -	  ssdpd is a stand-alone UNIX, no external dependencies but the
> -	  standard C library.  It has a built-in web server for serving
> -	  the UPnP XML description which Windows use to present the
> -	  icon, by default an InternetGatewayDevice is announced.
> +	  The project consists of ssdpd and ssdp-scan.  The former is a
> +	  stand-alone UNIX daemon, it has no dependencies but a standard
> +	  C library.  It comes with built-in web server for serving the
> +	  UPnP XML description on port 1901, the XML is used by Windows
> +	  to present the icon, by default an InternetGatewayDevice is
> +	  announced.
>  
> -	  Also included is ssdp-scan, a tool similar to mdns-scan, which
> -	  continuously scans for SSDP capable hosts on the network.
> -	  Take care only to use for debugging since it scans the network
> -	  quite aggressively.
> +	  ssdp-scan is a tool, similar to mdns-scan, to find other SSDP
> +	  capable hosts on the network.  Take care to only use it for
> +	  debugging, or quicker device discovery operations, since it
> +	  scans the network quite aggressively.
>  
>  	  https://github.com/troglobit/ssdp-responder/
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck

[Joachim Wiberg]

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 package/ssdp-responder/S50ssdpd | 47 +++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 20 deletions(-)
 mode change 100755 => 100644 package/ssdp-responder/S50ssdpd

diff --git a/package/ssdp-responder/S50ssdpd b/package/ssdp-responder/S50ssdpd
old mode 100755
new mode 100644
index e33992be91..6874fdbb70
--- a/package/ssdp-responder/S50ssdpd
+++ b/package/ssdp-responder/S50ssdpd
@@ -1,25 +1,32 @@
 #!/bin/sh
 
-NAME=ssdpd
-PIDFILE=/var/run/$NAME.pid
-DAEMON=/usr/sbin/$NAME
-CFGFILE=/etc/default/$NAME
+DAEMON=ssdpd
+PIDFILE=/var/run/$DAEMON.pid
+CFGFILE=/etc/default/$DAEMON
 
 DAEMON_ARGS=""
 
 # Read configuration variable file if it is present
-[ -f $CFGFILE ] && . $CFGFILE
+# shellcheck source=/dev/null
+[ -r "$CFGFILE" ] && . "$CFGFILE"
+
+cmd()
+{
+	start-stop-daemon -q -p "$PIDFILE" -x "$DAEMON" "$@"
+	status=$?
+	[ $status -eq 0 ] && echo "OK" || echo "FAIL"
+
+	return $status
+}
 
 start() {
-	printf 'Starting %s: ' "$NAME"
-	start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS
-	[ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Starting %s: ' "$DAEMON"
+	cmd -S -- "$DAEMON_ARGS"
 }
 
 stop() {
-	printf 'Stopping %s: ' "$NAME"
-	start-stop-daemon -K -q -p $PIDFILE -x $DAEMON
-	[ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Stopping %s: ' "$DAEMON"
+	cmd -K
 }
 
 restart() {
@@ -28,15 +35,15 @@ restart() {
 }
 
 case "$1" in
-    start|stop|restart)
-	"$1"
-	;;
-    reload)
-	restart
-	;;
-    *)
-	echo "Usage: $0 {start|stop|restart|reload}"
-	exit 1
+	start|stop|restart)
+		"$1"
+		;;
+	reload)
+		restart
+		;;
+	*)
+		echo "Usage: $0 {start|stop|restart|reload}"
+		exit 1
 esac
 
 exit $?
-- 
2.34.1

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

Re: [Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck

[Thomas Petazzoni via buildroot]

On Mon, 31 Oct 2022 18:46:32 +0100
Joachim Wiberg <troglobit@gmail.com> wrote:

> +cmd()
> +{
> +	start-stop-daemon -q -p "$PIDFILE" -x "$DAEMON" "$@"
> +	status=$?
> +	[ $status -eq 0 ] && echo "OK" || echo "FAIL"
> +
> +	return $status
> +}

I don't think we're using this cmd construct anywhere else in the tree,
or did I miss some change in our coding style/policy?

>  
>  start() {
> -	printf 'Starting %s: ' "$NAME"
> -	start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS
> -	[ $? = 0 ] && echo "OK" || echo "FAIL"

This all looked matching our coding style. Why are you changing this?
>  case "$1" in
> -    start|stop|restart)
> -	"$1"
> -	;;
> -    reload)
> -	restart
> -	;;
> -    *)
> -	echo "Usage: $0 {start|stop|restart|reload}"
> -	exit 1
> +	start|stop|restart)
> +		"$1"
> +		;;
> +	reload)
> +		restart
> +		;;
> +	*)
> +		echo "Usage: $0 {start|stop|restart|reload}"
> +		exit 1

I'm not sure what is the recommended indentation style in our init
scripts. Tabs? Spaces?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck

[Joachim Wiberg]

Hi Thomas!

On Mon, Oct 31, 2022 at 21:49, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> On Mon, 31 Oct 2022 18:46:32 +0100
> Joachim Wiberg <troglobit@gmail.com> wrote:
>> +cmd()
>> +{
>> +	start-stop-daemon -q -p "$PIDFILE" -x "$DAEMON" "$@"
>> +	status=$?
>> +	[ $status -eq 0 ] && echo "OK" || echo "FAIL"
>> +
>> +	return $status
>> +}
> I don't think we're using this cmd construct anywhere else in the tree,
> or did I miss some change in our coding style/policy?

I mentioned it in the cover letter, but that information should have
been here in this patch.  Sorry about that.

It all started out with utils/check-package telling me I used $DAEMON
wrong.  While changing that I ended up with a final comment from it
that said I should also "run shellcheck and fix the warnings".

It in turn had several grievances which I took one by one.  In this one
I used the same construct as in package/smcroute/S41smcroute to work
around a warning about using `$?` instead of using an `if cmd; then ...`

>>  start() {
>> -	printf 'Starting %s: ' "$NAME"
>> -	start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS
>> -	[ $? = 0 ] && echo "OK" || echo "FAIL"
>
> This all looked matching our coding style. Why are you changing this?
>>  case "$1" in
>> -    start|stop|restart)
>> -	"$1"
>> -	;;
>> -    reload)
>> -	restart
>> -	;;
>> -    *)
>> -	echo "Usage: $0 {start|stop|restart|reload}"
>> -	exit 1
>> +	start|stop|restart)
>> +		"$1"
>> +		;;
>> +	reload)
>> +		restart
>> +		;;
>> +	*)
>> +		echo "Usage: $0 {start|stop|restart|reload}"
>> +		exit 1
>
> I'm not sure what is the recommended indentation style in our init
> scripts. Tabs? Spaces?

Shellcheck pointed out this section had four spaces indent instead of
the eight (tab) used for the rest of the script.  Iirc from the "own
code coding style" discussions C and shell script should follow the
same style, but I may very well be wrong here.


Best regards
 /Joachim

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

Re: [Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck

[Yann E. MORIN]

Joachim, All,

On 2022-11-01 08:30 +0100, Joachim Wiberg spake thusly:
> On Mon, Oct 31, 2022 at 21:49, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> > On Mon, 31 Oct 2022 18:46:32 +0100
> > Joachim Wiberg <troglobit@gmail.com> wrote:
> >> +cmd()
> >> +{
> >> +	start-stop-daemon -q -p "$PIDFILE" -x "$DAEMON" "$@"
> >> +	status=$?
> >> +	[ $status -eq 0 ] && echo "OK" || echo "FAIL"
> >> +
> >> +	return $status
> >> +}
> > I don't think we're using this cmd construct anywhere else in the tree,
> > or did I miss some change in our coding style/policy?
> 
> I mentioned it in the cover letter, but that information should have
> been here in this patch.  Sorry about that.
> 
> It all started out with utils/check-package telling me I used $DAEMON
> wrong.  While changing that I ended up with a final comment from it
> that said I should also "run shellcheck and fix the warnings".
> 
> It in turn had several grievances which I took one by one.  In this one
> I used the same construct as in package/smcroute/S41smcroute to work
> around a warning about using `$?` instead of using an `if cmd; then ...`

We hanve cmd() in only two pacjages so far, smcroute and watchdogd, both
proided by you, so I can see you are aiming for some consistency! :-)

However, the majority of our SNNfoo startup scripts do not use this
cmd() wrapper construct, so I am not a fan of it.

(I like that it is generic and that we could have in a library of helpers
shared across our startup script, but IIRC we had a similar discussion a
long time ago, and decide against it, because it is trivial enough to
call start-stop-daemon).

The rest of the changes are however interesting, so could you respin
with just fixing those, pelase?

Note: it is acceptable that some shellcheck triggers get forcefully
disabled. For example, SC2086 (Double quote to prevent globbing and word
splitting) must be disabled when expanding $DAEMON_ARGS; this can be
achieved with:

    # shellcheck source=/dev/null
    [ -f $CFGFILE ] && . $CFGFILE

    # shellcheck disable=SC2086
    start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS

Basically, the canonical reference for a startup script is
package/busybox/S01syslogd.

Oh, btw, as much as I hate TABs, we do use TABs for indentation...

https://nightly.buildroot.org/#adding-packages-start-script

Regards,
Yann E. MORIN.

> >>  start() {
> >> -	printf 'Starting %s: ' "$NAME"
> >> -	start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS
> >> -	[ $? = 0 ] && echo "OK" || echo "FAIL"
> >
> > This all looked matching our coding style. Why are you changing this?
> >>  case "$1" in
> >> -    start|stop|restart)
> >> -	"$1"
> >> -	;;
> >> -    reload)
> >> -	restart
> >> -	;;
> >> -    *)
> >> -	echo "Usage: $0 {start|stop|restart|reload}"
> >> -	exit 1
> >> +	start|stop|restart)
> >> +		"$1"
> >> +		;;
> >> +	reload)
> >> +		restart
> >> +		;;
> >> +	*)
> >> +		echo "Usage: $0 {start|stop|restart|reload}"
> >> +		exit 1
> >
> > I'm not sure what is the recommended indentation style in our init
> > scripts. Tabs? Spaces?
> 
> Shellcheck pointed out this section had four spaces indent instead of
> the eight (tab) used for the rest of the script.  Iirc from the "own
> code coding style" discussions C and shell script should follow the
> same style, but I may very well be wrong here.
> 
> 
> Best regards
>  /Joachim
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/3] package/ssdp-responder: fix warnings from check-package and shellcheck

[Joachim Wiberg]

On Sat, Nov 05, 2022 at 19:44, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> On 2022-11-01 08:30 +0100, Joachim Wiberg spake thusly:
>> On Mon, Oct 31, 2022 at 21:49, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
>> > On Mon, 31 Oct 2022 18:46:32 +0100 Joachim Wiberg <troglobit@gmail.com> wrote:
>> >> +cmd()
>> >> +{
>> >> [SNIP]
>> >> +}
>> > I don't think we're using this cmd construct anywhere else in the tree,
>> > or did I miss some change in our coding style/policy?
>> 
>> I mentioned it in the cover letter, but that information should have
>> been here in this patch.  Sorry about that.
>> 
>> It all started out with utils/check-package telling me I used $DAEMON
>> wrong.  While changing that I ended up with a final comment from it
>> that said I should also "run shellcheck and fix the warnings".
>> 
>> It in turn had several grievances which I took one by one.  In this one
>> I used the same construct as in package/smcroute/S41smcroute to work
>> around a warning about using `$?` instead of using an `if cmd; then ...`
> We hanve cmd() in only two pacjages so far, smcroute and watchdogd, both
> proided by you, so I can see you are aiming for some consistency! :-)
> However, the majority of our SNNfoo startup scripts do not use this
> cmd() wrapper construct, so I am not a fan of it.
> (I like that it is generic and that we could have in a library of helpers
> shared across our startup script, but IIRC we had a similar discussion a
> long time ago, and decide against it, because it is trivial enough to
> call start-stop-daemon).

OK, I get it.  I was sort of aiming for a rehash of the start() and
stop() helper functions that many other start scripts use.  It's a small
generalization step up from that to reduce duplication and thus avoid
annoying bugs in behavior between stop and start actions.  Anyway ...

> The rest of the changes are however interesting, so could you respin
> with just fixing those, pelase?

Sure thing, I'll send a v2! :-)

> Note: it is acceptable that some shellcheck triggers get forcefully
> disabled.

OK, was a bit unsure about the policy for that. Thanks!

> For example, SC2086 (Double quote to prevent globbing and word
> splitting) must be disabled when expanding $DAEMON_ARGS; this can be
> achieved with:
>
>     # shellcheck source=/dev/null
>     [ -f $CFGFILE ] && . $CFGFILE
>
>     # shellcheck disable=SC2086
>     start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS

Ah, yes that makes sense.  Thank you!

> Basically, the canonical reference for a startup script is
> package/busybox/S01syslogd.

Got it! :)

> Oh, btw, as much as I hate TABs, we do use TABs for indentation...
> https://nightly.buildroot.org/#adding-packages-start-script

Yup, I'll be migrating my scripts to this.  Thanks for the
clarification and the thorough review!

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

[Buildroot] [PATCH v2 1/1] package/ssdp-responder: fix warnings from check-package and shellcheck

[Joachim Wiberg]

Summary of changes:

 - Fix use of $DAEMON, found by check-package
   - Expects DAEMON to be name of daemon controlled by script, this
     causes ripple efects in rest of script
   - Recommend `chmod a-x`, .mk file installs with `-m 0755`
 - Fix shellcheck warnings:
   - Use "$VAR" in case of spaces in filenames
   - recommend not using $? in if stmt, should use `if start-stop ...`
   - mismatch in indentation in case-esac

Changes since v1:

 - Revert introduction of cmd() wrapper for start-stop-daemon, instead
   call start-stop-daemon directly in start() and stop() functions

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 package/ssdp-responder/S50ssdpd | 47 +++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 20 deletions(-)
 mode change 100755 => 100644 package/ssdp-responder/S50ssdpd

diff --git a/package/ssdp-responder/S50ssdpd b/package/ssdp-responder/S50ssdpd
old mode 100755
new mode 100644
index e33992be91..8654de4f26
--- a/package/ssdp-responder/S50ssdpd
+++ b/package/ssdp-responder/S50ssdpd
@@ -1,25 +1,32 @@
 #!/bin/sh
 
-NAME=ssdpd
-PIDFILE=/var/run/$NAME.pid
-DAEMON=/usr/sbin/$NAME
-CFGFILE=/etc/default/$NAME
+DAEMON=ssdpd
+PIDFILE=/var/run/$DAEMON.pid
+CFGFILE=/etc/default/$DAEMON
 
 DAEMON_ARGS=""
 
 # Read configuration variable file if it is present
-[ -f $CFGFILE ] && . $CFGFILE
+# shellcheck source=/dev/null
+[ -r "$CFGFILE" ] && . "$CFGFILE"
 
+# shellcheck disable=SC2086
 start() {
-	printf 'Starting %s: ' "$NAME"
-	start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $DAEMON_ARGS
-	[ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Starting %s: ' "$DAEMON"
+	if start-stop-daemon -S -q -p "$PIDFILE" -x "$DAEMON" -- $DAEMON_ARGS; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
 }
 
 stop() {
-	printf 'Stopping %s: ' "$NAME"
-	start-stop-daemon -K -q -p $PIDFILE -x $DAEMON
-	[ $? = 0 ] && echo "OK" || echo "FAIL"
+	printf 'Stopping %s: ' "$DAEMON"
+	if start-stop-daemon -K -q -p "$PIDFILE" -x "$DAEMON"; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
 }
 
 restart() {
@@ -28,15 +35,15 @@ restart() {
 }
 
 case "$1" in
-    start|stop|restart)
-	"$1"
-	;;
-    reload)
-	restart
-	;;
-    *)
-	echo "Usage: $0 {start|stop|restart|reload}"
-	exit 1
+	start|stop|restart)
+		"$1"
+		;;
+	reload)
+		restart
+		;;
+	*)
+		echo "Usage: $0 {start|stop|restart|reload}"
+		exit 1
 esac
 
 exit $?
-- 
2.34.1

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

Re: [Buildroot] [PATCH v2 1/1] package/ssdp-responder: fix warnings from check-package and shellcheck

[Thomas Petazzoni via buildroot]

On Sun,  6 Nov 2022 19:19:46 +0100
Joachim Wiberg <troglobit@gmail.com> wrote:

> Summary of changes:
> 
>  - Fix use of $DAEMON, found by check-package
>    - Expects DAEMON to be name of daemon controlled by script, this
>      causes ripple efects in rest of script
>    - Recommend `chmod a-x`, .mk file installs with `-m 0755`
>  - Fix shellcheck warnings:
>    - Use "$VAR" in case of spaces in filenames
>    - recommend not using $? in if stmt, should use `if start-stop ...`
>    - mismatch in indentation in case-esac
> 
> Changes since v1:
> 
>  - Revert introduction of cmd() wrapper for start-stop-daemon, instead
>    call start-stop-daemon directly in start() and stop() functions

The changes between versions should be...

> 
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
> ---

... here. So that they don't show up in the final commit log.

Applied to next with this fixed. Thanks!

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot