Patchwork openssh: fix tab/spacing in init script

login
register
mail settings
Submitter Danomi Manchego
Date Aug. 21, 2013, 1:11 a.m.
Message ID <1377047460-1937-1-git-send-email-danomimanchego123@gmail.com>
Download mbox | patch
Permalink /patch/268660/
State Accepted
Commit 0be2fe9a8c07d1f58ca9eb288cb75aaae40550cd
Headers show

Comments

Danomi Manchego - Aug. 21, 2013, 1:11 a.m.
Several of the lines in S50sshd script have a strange mix of spaces
and tabs, that at least do not look consistent with neighboring lines.
This patch makes the spacing consistent, and also strips the trailing
spaces.

Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
---
 package/openssh/S50sshd |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
Thomas De Schampheleire - Aug. 21, 2013, 3:55 a.m.
Hi Danomi,

Op 21-aug.-2013 03:11 schreef "Danomi Manchego" <danomimanchego123@gmail.com>
het volgende:
>
> Several of the lines in S50sshd script have a strange mix of spaces
> and tabs, that at least do not look consistent with neighboring lines.
> This patch makes the spacing consistent, and also strips the trailing
> spaces.
>
> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
> ---

I haven't checked if there are such problems, but have you considered
checking all init scripts for the same type of problems and fixing them in
this same patch?

A rough way of finding these scripts is:
find -name "S*"

Thanks,
Thomas
Danomi Manchego - Aug. 21, 2013, 1:39 p.m.
On Tue, Aug 20, 2013 at 11:39 PM, Thomas De Schampheleire <
patrickdepinguin@gmail.com> wrote:

> I haven't checked if there are such problems, but have you considered
> checking all init scripts for the same type of problems and fixing them in
> this same patch?
>

Thomas,

I looked through the other init scripts.  There's some variation, mostly in:

* Tabs vs spacing.
* Where spaces are used instead of tabs, sometimes two spaces are used per
level, other times four.
* Indentation of start/stop under the case "$1"  /  esac clause.
    * Some of the start/stop begin in the first column, aligned with 'case'.
    * Some of them have two spaces, followed by actions offset with a tabs.
    * Some of them have two spaces, followed by actions offset with a
sapces.
    * Some are offset with tab, followed by actions offset with two tabs.

So I took the approach of leaving that part alone, and just fixing the
lines that were offset by a mix of spaces and tabs, since that seemed
clearly bad.

I'd be willing to go through all of them and make them uniform, if
recommended guidelines can be stated with regards to the above variations.
 Tabs are tempting only because it's a single character in text files that
go in the target - otherwise, I'm normally in the tabs-are-evil camp, since
thing which start out with tabs tend get modified with spaces over time,
and because editor rendering of tab width can vary.  So I'd be willing to
go either way.

Danomi -
Thomas De Schampheleire - Aug. 22, 2013, 1:59 p.m.
Hi Danomi,

On Wed, Aug 21, 2013 at 3:39 PM, Danomi Manchego
<danomimanchego123@gmail.com> wrote:
> On Tue, Aug 20, 2013 at 11:39 PM, Thomas De Schampheleire
> <patrickdepinguin@gmail.com> wrote:
>>
>> I haven't checked if there are such problems, but have you considered
>> checking all init scripts for the same type of problems and fixing them in
>> this same patch?
>
>
> Thomas,
>
> I looked through the other init scripts.  There's some variation, mostly in:
>
> * Tabs vs spacing.
> * Where spaces are used instead of tabs, sometimes two spaces are used per
> level, other times four.
> * Indentation of start/stop under the case "$1"  /  esac clause.
>     * Some of the start/stop begin in the first column, aligned with 'case'.
>     * Some of them have two spaces, followed by actions offset with a tabs.
>     * Some of them have two spaces, followed by actions offset with a
> sapces.
>     * Some are offset with tab, followed by actions offset with two tabs.
>
> So I took the approach of leaving that part alone, and just fixing the lines
> that were offset by a mix of spaces and tabs, since that seemed clearly bad.
>
> I'd be willing to go through all of them and make them uniform, if
> recommended guidelines can be stated with regards to the above variations.
> Tabs are tempting only because it's a single character in text files that go
> in the target - otherwise, I'm normally in the tabs-are-evil camp, since
> thing which start out with tabs tend get modified with spaces over time, and
> because editor rendering of tab width can vary.  So I'd be willing to go
> either way.

Thanks for checking. Seems there was more variance than I expected.

I'm not sure what to do here. Unless such init scripts have been
copied from elsewhere (in which making cosmetic changes may be
undesirable), I'm ok with having them changed according to a given
guideline.

Before trying to establish such a guideline, what does the community
think about this?
If there is no consensus in changing, then no need for a guideline.

Thanks,
Thomas
Arnout Vandecappelle - Aug. 22, 2013, 4:11 p.m.
On 22/08/13 15:59, Thomas De Schampheleire wrote:
>> >I'd be willing to go through all of them and make them uniform, if
>> >recommended guidelines can be stated with regards to the above variations.
>> >Tabs are tempting only because it's a single character in text files that go
>> >in the target - otherwise, I'm normally in the tabs-are-evil camp, since
>> >thing which start out with tabs tend get modified with spaces over time, and
>> >because editor rendering of tab width can vary.  So I'd be willing to go
>> >either way.
> Thanks for checking. Seems there was more variance than I expected.
>
> I'm not sure what to do here. Unless such init scripts have been
> copied from elsewhere (in which making cosmetic changes may be
> undesirable), I'm ok with having them changed according to a given
> guideline.

  If it is copied from elsewhere then it probably shouldn't be in 
buildroot, but just copied from elsewhere.


> Before trying to establish such a guideline, what does the community
> think about this?
> If there is no consensus in changing, then no need for a guideline.

  I guess that's a judgement call for Peter to make.

  Note that we also have this kind of whitespace inconsistencies in e.g. 
support/scripts.


  Regards,
  Arnout
Peter Korsgaard - Aug. 27, 2013, 8:40 p.m.
>>>>> "Danomi" == Danomi Manchego <danomimanchego123@gmail.com> writes:

 Danomi> Several of the lines in S50sshd script have a strange mix of spaces
 Danomi> and tabs, that at least do not look consistent with neighboring lines.
 Danomi> This patch makes the spacing consistent, and also strips the trailing
 Danomi> spaces.

 Danomi> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>

Committed, thanks.

Patch

diff --git a/package/openssh/S50sshd b/package/openssh/S50sshd
index 89f1b9a..b65b3c8 100644
--- a/package/openssh/S50sshd
+++ b/package/openssh/S50sshd
@@ -23,7 +23,7 @@  if [ ! -f /etc/ssh_host_dsa_key ] ; then
 	echo Generating DSA Key...
 	echo THIS CAN TAKE A MINUTE OR TWO DEPENDING ON YOUR PROCESSOR!
 	echo
-        /usr/bin/ssh-keygen -t dsa -f /etc/ssh_host_dsa_key -C '' -N ''
+	/usr/bin/ssh-keygen -t dsa -f /etc/ssh_host_dsa_key -C '' -N ''
 fi
 
 # Check for the SSH2 ECDSA key
@@ -33,35 +33,35 @@  if [ ! -f /etc/ssh_host_ecdsa_key ]; then
 	echo
 	/usr/bin/ssh-keygen -t ecdsa -f /etc/ssh_host_ecdsa_key -C '' -N ''
 fi
-                
+
 umask 077
 
 start() {
- 	echo -n "Starting sshd: "
+	echo -n "Starting sshd: "
 	/usr/sbin/sshd
 	touch /var/lock/sshd
 	echo "OK"
-}	
+}
 stop() {
 	echo -n "Stopping sshd: "
-        killall	sshd 
+	killall sshd
 	rm -f /var/lock/sshd
-	echo "OK" 
+	echo "OK"
 }
 restart() {
 	stop
 	start
-}	
+}
 
 case "$1" in
   start)
-  	start
+	start
 	;;
   stop)
-  	stop
+	stop
 	;;
   restart|reload)
-  	restart
+	restart
 	;;
   *)
 	echo "Usage: $0 {start|stop|restart}"