Searching a colon-separated list of directories

moraxu · 02-23-2013, 02:53 PM

Hello,

I was given this exercise:

Quote:

Write a function named funload that takes as its single argument the name of a file containing other functions. The purpose of funload is to make all functions in the named file available in the current shell; that is, funload loads the functions from the named file. To locate the file, funload searches the colon-separated list of directories given by the environment variable FUNPATH. Assume that the format of FUNPATH is the same as PATH and that searching FUNPATH is similar to the shell’s search of the PATH variable.

Below is my function. Do you have any remarks regarding it? Does something can be simplified?

Code:

function funload()
{
	if [ $# -ne 1 ]
	then
		echo "Invalid arguments!" 1>&2
		echo "Usage: funload simple_filename" 1>&2
		return 1
	fi

	if [ -z "$1" ]
	then
		echo "Filename is an empty string!" 1>&2
		echo "Usage: funload simple_filename" 1>&2
		return 1
	fi
	
	if [ $(basename "$1") != "$1" ]
	then
		echo "Filename isn't a simple filename!" 1>&2
		echo "Usage: funload simple_filename" 1>&2
		return 1
	fi
	
	if [[ -z "$FUNPATH" || "$FUNPATH" =~ ^\ +$ ]]
	then
		echo "Environment variable FUNPATH is unset or empty!" 1>&2
		return 1
	fi

	# After tests, I assume that every pathname
	# in the variable is an absolute pathname
	
	local current_path all_paths=$FUNPATH prev_all_paths

	while [ "$all_paths" != "$prev_all_paths" ]
	do
		current_path=${all_paths%%:*}

		if [ -z "$current_path" ]
		then
			[ -e ~/"$1" ] && { . ~/"$1" ; return ; }
		else
			[ -e "$current_path/$1" ] && { . "$current_path/$1" ; return ; }
		fi
		
		prev_all_paths=$all_paths
		all_paths=${all_paths#*:}
		
		if [ -z "$all_paths" ]
		then
			[ -e ~/"$1" ] && { . ~/"$1" ; return ; }
		fi
		
	done
}

rigor · 02-23-2013, 03:29 PM

Whether or not these suggestions are appropriate to your situation can naturally depend on what limitations your instructor may impose.

I would tend to have a single return code for success, but different return codes for different failures. That's so the calling program can determine the nature of the failure, if it needs to do so.

If I'm going to output a fixed message, in which I don't want any substitutions, I would tend to use single quotes ( with proper adjustments for any included "apostrophes" ).

If the usage message is always the same, rather than repeat the same code again and again, to output the usage message, I'd be prone to make a usage() function, then call the usage() function. Or in this case, since it appears to be that funload() might be a function that is to be made available directly in the shell's environment, I'd probably name the usage() function something like funload_usage().

Although it may not be a great idea from the standpoint of security, strictly speaking, the PATH can contain relative path's, such as "dot". So I might not assume that each "pathname" is absolute, if you are meant to process FUNPATH as the shell would process PATH.

David the H. · 02-24-2013, 07:53 PM

Just to be clear, is this a bash script, a POSIX-compliant sh script, or something else? The syntax you're using appears to be bash.

1) POSIX function declaration syntax is "funcname()". In bash you can also use "function funcname". Use one or the other, not both. Although bash won't complain if you do, other shells may not be so nice.

2) If possible, I would instead first split the FUNPATH into an array. Then you can simply use a for loop to iterate over the entries for the search. Use a break command to exit the loop when/if a match is found.

3) Instead of multiple separate if statements, you should probably combine them into a single if..elif..else.

4) basename can be replaced with a simple parameter substitution. ( "${path##*/}" )

5) Finally, be consistent. If the shell supports "[[..]]", then use that exclusively. Except also that in bash/ksh, numeric comparisons should be done with "((..))" instead.

http://mywiki.wooledge.org/BashFAQ/031
http://mywiki.wooledge.org/ArithmeticExpression

moraxu · 02-25-2013, 10:42 AM

OK, thanks for answers. Yes, it's a bash script.

Quote:

Originally Posted by David the H.

If possible, I would instead first split the FUNPATH into an array. Then you can simply use a for loop to iterate over the entries for the search. Use a break command to exit the loop when/if a match is found.

I was going to do so, but after considering that pathname might contain a whitespace, dropped this idea.

grail · 02-25-2013, 11:26 AM

Quote:

I was going to do so, but after considering that pathname might contain a whitespace, dropped this idea.

This need only be a concern if not dealt with. Try considering what determines how an array is built and look at how you might change the necessary delimiter to your needs.

David the H. · 02-26-2013, 07:05 AM

Indeed. The possibility of encountering whitespace is one of the best reasons why you should use arrays for lists of things. The only question comes in loading it properly in the first place.

You'll find the answer to that in these two links:

How can I use array variables?
http://mywiki.wooledge.org/BashFAQ/005/

How do I do string manipulations in bash?
http://mywiki.wooledge.org/BashFAQ/100