[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Andreas Ehmanns]

Signed-off-by: Andreas Ehmanns <universeII@gmx.de>
---
 package/netsnmp/S59snmpd | 98 +++++++++++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/package/netsnmp/S59snmpd b/package/netsnmp/S59snmpd
index 4eea512..94773db 100755
--- a/package/netsnmp/S59snmpd
+++ b/package/netsnmp/S59snmpd
@@ -1,4 +1,4 @@
-#! /bin/sh -e
+#! /bin/sh
 ### BEGIN INIT INFO
 # Provides:           snmpd snmptrapd
 # Required-Start:     $network $local_fs
@@ -11,7 +11,6 @@
 #
 # Author:    Jochen Friedrich <jochen@scram.de>
 #
-set -e
 
 export PATH=/sbin:/usr/sbin:/bin:/usr/bin
 
@@ -38,56 +37,71 @@ if [ "$SNMPDCOMPAT" = "yes" ]; then
   ln -sf /var/agentx/master /var/run/agentx
 fi
 
-case "$1" in
-  start)
-    printf "Starting network management services:"
+start() {
     if [ "$SNMPDRUN" = "yes" -a -f /etc/snmp/snmpd.conf ]; then
-      start-stop-daemon -q -S -x /usr/sbin/snmpd -- $SNMPDOPTS
-      printf " snmpd"
+        printf "Starting SNMP daemon: "
+        start-stop-daemon -q -S -x /usr/sbin/snmpd -- $SNMPDOPTS
+        [ $? = 0 ] && echo "OK" || echo "FAIL"
     fi
+
     if [ "$TRAPDRUN" = "yes" -a -f /etc/snmp/snmptrapd.conf ]; then
-      start-stop-daemon -q -S -x /usr/sbin/snmptrapd -- $TRAPDOPTS
-      printf " snmptrapd"
+        printf "Starting SNMP trap daemon: "
+        start-stop-daemon -q -S -x /usr/sbin/snmptrapd -- $TRAPDOPTS
+        [ $? = 0 ] && echo "OK" || echo "FAIL"
     fi
-    echo "."
-    ;;
-  stop)
-    printf "Stopping network management services:"
-    start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmpd
-    printf " snmpd"
-    start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmptrapd
-    printf " snmptrapd"
-    echo "."
-    ;;
-  restart)
-    printf "Restarting network management services:"
-    start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmpd
-    start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmptrapd
-    # Allow the daemons time to exit completely.
-    sleep 2
+}
+
+stop() {
     if [ "$SNMPDRUN" = "yes" -a -f /etc/snmp/snmpd.conf ]; then
-      start-stop-daemon -q -S -x /usr/sbin/snmpd -- $SNMPDOPTS
-      printf " snmpd"
+        printf "Stopping SNMP daemon: "
+        start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmpd
+        [ $? = 0 ] && echo "OK" || echo "FAIL"
     fi
+
     if [ "$TRAPDRUN" = "yes" -a -f /etc/snmp/snmptrapd.conf ]; then
-      # Allow snmpd time to start up.
-      sleep 1
-      start-stop-daemon -q -S -x /usr/sbin/snmptrapd -- $TRAPDOPTS
-      printf " snmptrapd"
+        printf "Stopping SNMP trap daemon: "
+        start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmptrapd
+        [ $? = 0 ] && echo "OK" || echo "FAIL"
     fi
-    echo "."
-    ;;
-  reload|force-reload)
-    printf "Reloading network management services:"
+}
+
+reload() {
     if [ "$SNMPDRUN" = "yes" -a -f /etc/snmp/snmpd.conf ]; then
-      start-stop-daemon -q -K -s 1 -p /var/run/snmpd.pid -x /usr/sbin/snmpd
-      printf " snmpd"
+        printf "Reloading SNMP daemon: "
+        start-stop-daemon -q -K -s 1 -p /var/run/snmpd.pid -x /usr/sbin/snmpd
+       [ $? = 0 ] && echo "OK" || echo "FAIL" 
+    fi
+
+    if [ "$TRAPDRUN" = "yes" -a -f /etc/snmp/snmptrapd.conf ]; then
+        printf "Reloading SNMP trap daemon: "
+        start-stop-daemon -q -K -s 1 -p /var/run/snmptrapd.pid -x /usr/sbin/snmptrapd
+        [ $? = 0 ] && echo "OK" || echo "FAIL"
     fi
-    echo "."
-    ;;
-  *)
-    echo "Usage: /etc/init.d/snmpd {start|stop|restart|reload|force-reload}"
-    exit 1
+}
+
+case "$1" in
+    start)
+        start
+        ;;
+
+    stop)
+        stop
+        ;;
+
+    restart)
+        stop
+        # Allow the daemons time to exit completely.
+        sleep 2
+        start
+        ;;
+
+    reload|force-reload)
+        reload
+        ;;
+
+    *)
+        echo "Usage: $0 {start|stop|restart|reload|force-reload}"
+        exit 1
 esac
 
 exit 0
-- 
2.1.4

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Thomas Petazzoni]

Dear Andreas Ehmanns,

On Wed, 24 Feb 2016 18:31:21 +0100, Andreas Ehmanns wrote:
> Signed-off-by: Andreas Ehmanns <universeII@gmx.de>
> ---
>  package/netsnmp/S59snmpd | 98 +++++++++++++++++++++++++++---------------------
>  1 file changed, 56 insertions(+), 42 deletions(-)

While your patch looks OK, it is doing *much* more than "adding
OK/FAIL" as your commit title indicates.

Ideally, you should separate your changes into multiple patches to make
it easier to review the changes (like one patch to change to use
functions, one patch to add OK/FAIL, one patch to remove set -e, etc.).
Or at least, write a commit log that details all the changes that are
being done.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Andreas Ehmanns]

Dear Thomas,
thanks for the feedback. I will prepare several patches including 
smaller changes and re-submit them.

Regards,
Andreas

Am 25.02.2016 um 10:46 schrieb Thomas Petazzoni:
> Dear Andreas Ehmanns,
>
> On Wed, 24 Feb 2016 18:31:21 +0100, Andreas Ehmanns wrote:
>> Signed-off-by: Andreas Ehmanns <universeII@gmx.de>
>> ---
>>   package/netsnmp/S59snmpd | 98 +++++++++++++++++++++++++++---------------------
>>   1 file changed, 56 insertions(+), 42 deletions(-)
> While your patch looks OK, it is doing *much* more than "adding
> OK/FAIL" as your commit title indicates.
>
> Ideally, you should separate your changes into multiple patches to make
> it easier to review the changes (like one patch to change to use
> functions, one patch to add OK/FAIL, one patch to remove set -e, etc.).
> Or at least, write a commit log that details all the changes that are
> being done.
>
> Thanks a lot!
>
> Thomas

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Thomas Petazzoni]

Andreas,

On Thu, 25 Feb 2016 21:02:07 +0100, Andreas Ehmanns wrote:

> thanks for the feedback. I will prepare several patches including 
> smaller changes and re-submit them.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Andreas Ehmanns]

Thomas,
one "stupid" questions. Shall the patches be incremental or relative to 
the original file?
Example: Change some lines and create the first patch. Then change the 
next things based on the original file or based on the already patched file?

Regards,
Andreas

Am 25.02.2016 um 21:25 schrieb Thomas Petazzoni:
> Andreas,
>
> On Thu, 25 Feb 2016 21:02:07 +0100, Andreas Ehmanns wrote:
>
>> thanks for the feedback. I will prepare several patches including
>> smaller changes and re-submit them.
> Thanks a lot!
>
> Thomas

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Thomas Petazzoni]

Hello,

On Thu, 25 Feb 2016 21:43:48 +0100, Andreas Ehmanns wrote:

> one "stupid" questions. Shall the patches be incremental or relative to 
> the original file?
> Example: Change some lines and create the first patch. Then change the 
> next things based on the original file or based on the already patched file?

Based on the already patched file of course, otherwise you won't go
very far. You need to create a patch series. To do this, create a Git
branch, make a set of commits that stack on top of each other, and then
use git format-patch to create one patch file for each commit in your
branch, and send them with git send-mail.

You can look in the Buildroot manual, there are some explanations about
this:

   http://nightly.buildroot.org/#submitting-patches

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] Added OK/FAIL output to SNMP init script.

[Andreas Ehmanns]

Thomas,
thanks for the clarification.

Regards,
Andreas

Am 25.02.2016 um 22:04 schrieb Thomas Petazzoni:
> Hello,
>
> On Thu, 25 Feb 2016 21:43:48 +0100, Andreas Ehmanns wrote:
>
>> one "stupid" questions. Shall the patches be incremental or relative to
>> the original file?
>> Example: Change some lines and create the first patch. Then change the
>> next things based on the original file or based on the already patched file?
> Based on the already patched file of course, otherwise you won't go
> very far. You need to create a patch series. To do this, create a Git
> branch, make a set of commits that stack on top of each other, and then
> use git format-patch to create one patch file for each commit in your
> branch, and send them with git send-mail.
>
> You can look in the Buildroot manual, there are some explanations about
> this:
>
>     http://nightly.buildroot.org/#submitting-patches
>
> Best regards,
>
> Thomas