bash how to improve this script

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
4
down vote

favorite












I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:



I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:



$USER is defined as somebody who logs into the system



My solution (it works) is:



$IGNORE_USER="USER1 USER2 USER3"

if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
 do
 if $USER = $usr; then
 exit_program "bye bye" 
 fi
 done
fi



bash shell-script




share|improve this question



















  • You don't need to loop over your array of users. See here
    – xenoid
    10 hours ago










  • If you have a working script and want it reviewed, consider codereview.stackexchange.com
    – kojiro
    7 hours ago
















up vote
4
down vote

favorite












I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:



I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:



$USER is defined as somebody who logs into the system



My solution (it works) is:



$IGNORE_USER="USER1 USER2 USER3"

if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
 do
 if $USER = $usr; then
 exit_program "bye bye" 
 fi
 done
fi



bash shell-script




share|improve this question



















  • You don't need to loop over your array of users. See here
    – xenoid
    10 hours ago










  • If you have a working script and want it reviewed, consider codereview.stackexchange.com
    – kojiro
    7 hours ago












up vote
4
down vote

favorite









up vote
4
down vote

favorite











I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:



I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:



$USER is defined as somebody who logs into the system



My solution (it works) is:



$IGNORE_USER="USER1 USER2 USER3"

if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
 do
 if $USER = $usr; then
 exit_program "bye bye" 
 fi
 done
fi



bash shell-script




share|improve this question











I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:



I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:



$USER is defined as somebody who logs into the system



My solution (it works) is:



$IGNORE_USER="USER1 USER2 USER3"

if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
 do
 if $USER = $usr; then
 exit_program "bye bye" 
 fi
 done
fi




bash shell-script





share|improve this question










share|improve this question




share|improve this question









asked 13 hours ago









bernhard

212




212











  • You don't need to loop over your array of users. See here
    – xenoid
    10 hours ago










  • If you have a working script and want it reviewed, consider codereview.stackexchange.com
    – kojiro
    7 hours ago
















  • You don't need to loop over your array of users. See here
    – xenoid
    10 hours ago










  • If you have a working script and want it reviewed, consider codereview.stackexchange.com
    – kojiro
    7 hours ago















You don't need to loop over your array of users. See here
– xenoid
10 hours ago




You don't need to loop over your array of users. See here
– xenoid
10 hours ago












If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago




If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago










2 Answers
2






active

oldest

votes

















up vote
7
down vote













That script does not work.



You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.



Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.



It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.



Suggestion:



ignore=( 'user1' 'user2' 'user3' )

for u in "$ignore[@]"; do
 if [ "$USER" = "$u" ]; then
 exit_program 'bye bye'
 fi
done


This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.



In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.



Related:



  • The ShellCheck website

For a version that is not specific to bash, but that would run in any POSIX-like shell:



set -- 'user1' 'user2' 'user3'

for u do
 if [ "$USER" = "$u" ]; then
 exit_program 'bye bye'
 fi
done


This uses the list of positional parameters as the list of usernames to ignore instead of an array.






share|improve this answer






























    up vote
    3
    down vote













    grep solution



    Use the "-w" feature of grep, to match a word.
    Daresay it won't work on older grep implementations, such as Solaris, AIX etc.



    echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'


    Try it online!



    bash internal solution



    Go entirely bash, and don't rely on grep.



    bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.



    [[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'


    Try it online!






    share|improve this answer























      Your Answer







      StackExchange.ready(function()
      var channelOptions =
      tags: "".split(" "),
      id: "106"
      ;
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function()
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled)
      StackExchange.using("snippets", function()
      createEditor();
      );

      else
      createEditor();

      );

      function createEditor()
      StackExchange.prepareEditor(
      heartbeatType: 'answer',
      convertImagesToLinks: false,
      noModals: false,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      );



      );








       

      draft saved


      draft discarded


















      StackExchange.ready(
      function ()
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2funix.stackexchange.com%2fquestions%2f460658%2fbash-how-to-improve-this-script%23new-answer', 'question_page');

      );

      Post as a guest

















      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes








      up vote
      7
      down vote













      That script does not work.



      You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
      There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.



      Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.



      It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.



      Suggestion:



      ignore=( 'user1' 'user2' 'user3' )

      for u in "$ignore[@]"; do
       if [ "$USER" = "$u" ]; then
       exit_program 'bye bye'
       fi
      done


      This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.



      In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.



      Related:



      • The ShellCheck website

      For a version that is not specific to bash, but that would run in any POSIX-like shell:



      set -- 'user1' 'user2' 'user3'

      for u do
       if [ "$USER" = "$u" ]; then
       exit_program 'bye bye'
       fi
      done


      This uses the list of positional parameters as the list of usernames to ignore instead of an array.






      share|improve this answer



























        up vote
        7
        down vote













        That script does not work.



        You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
        There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.



        Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.



        It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.



        Suggestion:



        ignore=( 'user1' 'user2' 'user3' )

        for u in "$ignore[@]"; do
         if [ "$USER" = "$u" ]; then
         exit_program 'bye bye'
         fi
        done


        This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.



        In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.



        Related:



        • The ShellCheck website

        For a version that is not specific to bash, but that would run in any POSIX-like shell:



        set -- 'user1' 'user2' 'user3'

        for u do
         if [ "$USER" = "$u" ]; then
         exit_program 'bye bye'
         fi
        done


        This uses the list of positional parameters as the list of usernames to ignore instead of an array.






        share|improve this answer

























          up vote
          7
          down vote










          up vote
          7
          down vote









          That script does not work.



          You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
          There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.



          Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.



          It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.



          Suggestion:



          ignore=( 'user1' 'user2' 'user3' )

          for u in "$ignore[@]"; do
           if [ "$USER" = "$u" ]; then
           exit_program 'bye bye'
           fi
          done


          This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.



          In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.



          Related:



          • The ShellCheck website

          For a version that is not specific to bash, but that would run in any POSIX-like shell:



          set -- 'user1' 'user2' 'user3'

          for u do
           if [ "$USER" = "$u" ]; then
           exit_program 'bye bye'
           fi
          done


          This uses the list of positional parameters as the list of usernames to ignore instead of an array.






          share|improve this answer















          That script does not work.



          You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
          There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.



          Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.



          It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.



          Suggestion:



          ignore=( 'user1' 'user2' 'user3' )

          for u in "$ignore[@]"; do
           if [ "$USER" = "$u" ]; then
           exit_program 'bye bye'
           fi
          done


          This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.



          In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.



          Related:



          • The ShellCheck website

          For a version that is not specific to bash, but that would run in any POSIX-like shell:



          set -- 'user1' 'user2' 'user3'

          for u do
           if [ "$USER" = "$u" ]; then
           exit_program 'bye bye'
           fi
          done


          This uses the list of positional parameters as the list of usernames to ignore instead of an array.







          share|improve this answer















          share|improve this answer



          share|improve this answer








          edited 8 hours ago


























          answered 12 hours ago









          Kusalananda

          100k13199310




          100k13199310






















              up vote
              3
              down vote













              grep solution



              Use the "-w" feature of grep, to match a word.
              Daresay it won't work on older grep implementations, such as Solaris, AIX etc.



              echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'


              Try it online!



              bash internal solution



              Go entirely bash, and don't rely on grep.



              bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.



              [[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'


              Try it online!






              share|improve this answer



























                up vote
                3
                down vote













                grep solution



                Use the "-w" feature of grep, to match a word.
                Daresay it won't work on older grep implementations, such as Solaris, AIX etc.



                echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'


                Try it online!



                bash internal solution



                Go entirely bash, and don't rely on grep.



                bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.



                [[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'


                Try it online!






                share|improve this answer

























                  up vote
                  3
                  down vote










                  up vote
                  3
                  down vote









                  grep solution



                  Use the "-w" feature of grep, to match a word.
                  Daresay it won't work on older grep implementations, such as Solaris, AIX etc.



                  echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'


                  Try it online!



                  bash internal solution



                  Go entirely bash, and don't rely on grep.



                  bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.



                  [[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'


                  Try it online!






                  share|improve this answer















                  grep solution



                  Use the "-w" feature of grep, to match a word.
                  Daresay it won't work on older grep implementations, such as Solaris, AIX etc.



                  echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'


                  Try it online!



                  bash internal solution



                  Go entirely bash, and don't rely on grep.



                  bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.



                  [[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'


                  Try it online!







                  share|improve this answer















                  share|improve this answer



                  share|improve this answer








                  edited 6 hours ago


























                  answered 8 hours ago









                  steve

                  11.8k22046




                  11.8k22046






















                       

                      draft saved


                      draft discarded


























                       


                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function ()
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2funix.stackexchange.com%2fquestions%2f460658%2fbash-how-to-improve-this-script%23new-answer', 'question_page');

                      );

                      Post as a guest
































































                      Comments

                      Post a Comment

                      Popular posts from this blog

                      What is the equation of a 3D cone with generalised tilt?

                      Image
                      Clash Royale CLAN TAG #URR8PPP up vote 2 down vote favorite What is the equation of a 3D cone with generalized tilt? I've noticed that in most equations given to represent a cone, there is no parameter which defines the tilt of the cone in 3D space and that most of them have their point at the origin $(0,0,0)$ - I was wondering if anyone could give me a more generalized cone equation for a cone in any position in 3D space 3d share | cite | improve this question edited Jul 25 '16 at 0:05 ervx 9,425 3 13 36 asked Jul 24 '16 at 15:38 Charlene 11 2 2 Welcome to Math.SE! Could you please explain a few things to help people give an answer useful to you: 1. What do you mean by "generalized tilt"? 2. Are you asking about a right circular cone? Does the angle at the vertex matter? 3. What form of answer (implicit, parametric...?) are you looking for? 4. If this is homework, what tools do you have available, an...
                      Read more

                      Relationship between determinant of matrix and determinant of adjoint?

                      Image
                      Clash Royale CLAN TAG #URR8PPP up vote 2 down vote favorite We are studying adjoints in class, and I was curious if there is a relationship between the determinant of matrix A, and the determinant of the adjoint of matrix A? I assume there would be a relationship because finding the adjoint requires creating a cofactor matrix and then transposing it. linear-algebra determinant adjoint-operators share | cite | improve this question asked Nov 17 '17 at 17:32 CluelessCoder 32 5 For a square matrix $A$ of order $n$, we have $A(operatornameadj A)=(operatornameadj A)A=|A|I_n$ where $I_n$ is the identity matrix of order $n$. This should tell you about the determinant of the adjoint in terms of that of $A$ (use multiplicative property and other elementary properties of determinants). – Prasun Biswas Nov 17 '17 at 17:35 1 @CluelessCoder: see here: math.stackexchange.com/questions/516127/…...
                      Read more

                      Color the edges and diagonals of a regular polygon

                      Image
                      Clash Royale CLAN TAG #URR8PPP up vote 5 down vote favorite 1 Here is the problem: For what $n$ is it possible to color the edges and diagonals of an $n$-side regular polygon with $dfracbinomn23$ colors, such that you use every color exactly three times and for every color the three segments (edges or diagonals) with that color form a triangle? Trivially $nequiv 0 text or 1 pmod3$. I can also prove that if the statement is true for $k$ than it is true for $3k$ as well. How to finish? Please help! Thanks combinatorial-geometry share | cite | improve this question edited Aug 6 at 21:57 alcana 118 4 asked Aug 6 at 21:15 Leo Gardner 356 11 Even assuming "polyhpn" in the title is a typo for polygon , it is unclear what you mean by "the edges and sides". Aren't the side of a regular polygon the same as the edges? A regular $n$-sided polygon has $n$ edges. – hardmath Aug 6 at 21:20 3 $n$ ne...
                      Read more