HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Generates a randomized locally-administered unicast MAC address

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
generatesadministeredaddresslocallyrandomizedunicastmac

Problem

This is the first time I've attempted writing something in bash, and would appreciate any feedback or criticism to make it better.

#! /bin/bash
macArr=( 0 0 0 0 0 0 )
for ((i=0; i >1)|1)<<1)"
    fi
    prependZero=$([ $byte -lt 16 ] && echo 1 || echo 0)
    # convert byte from decimal int to hex string
    byte=$(printf "%x" "$byte")
    # uppercase any letters in the string
    macArr[$i]="${byte^^}"
    if [ $prependZero -eq 1 ];then
        macArr[$i]="0${macArr[$i]}"
    fi
done
printf "%s" "${macArr[@]}"

Solution

Interesting idea, and pretty good first Bash script.

Bash arithmetics

Instead of this:

let "byte=$RANDOM%255"


I find this more readable:

((byte = RANDOM % 255))


You can use math like this within ((...)), without $ of variables, and with spaces around operators to keep the expressions nicely readable.

You can use this in conditions too,
in fact, instead of this:

if [ $i -eq 0 ];then


It's better to write like this:

if (( i == 0 )); then


And here, this is really awkward, as you are running a sub-shell, and storing the output of echo commands:

prependZero=$([ $byte -lt 16 ] && echo 1 || echo 0)


When it could be simply:

((prependZero = byte < 16))


Padding with 0

Since you already use printf to convert from integer to hexadecimal,
you can actually use it to 0-pad too:

byte=$(printf "%02x" "$byte")


This will pad with zeros on the left to make it 2-digits wide

Suggested implementation

With the above tips, the script can be simpler and cleaner.

#!/bin/bash

macArr=( 0 0 0 0 0 0 )
for ((i = 0; i > 1) | 1) << 1))
    fi

    # convert byte from decimal int to hex string
    byte=$(printf "%02x" "$byte")

    # uppercase any letters in the string
    macArr[$i]="${byte^^}"
done
printf "%s" "${macArr[@]}"

Code Snippets

let "byte=$RANDOM%255"
((byte = RANDOM % 255))
if [ $i -eq 0 ];then
if (( i == 0 )); then
prependZero=$([ $byte -lt 16 ] && echo 1 || echo 0)

Context

StackExchange Code Review Q#119550, answer score: 4

Revisions (0)

No revisions yet.