On Thursday 12 January 2012 01:49:59 Michał Górny wrote:
> On Wed, 11 Jan 2012 23:09:46 +0100 Ulrich Mueller wrote:
> > cdrom_get_cds() {
> >
> > # first we figure out how many cds we're dealing with by
> > # the # of files they gave us
> > local cdcnt=0
> > local f=
> > for f in "$@" ; do
> >
> > ((++cdcnt))
> > export CDROM_CHECK_${cdcnt}="$f"
> >
> > done
> > export CDROM_TOTAL_CDS=${cdcnt}
> > export CDROM_CURRENT_CD=1
>
> Why are you exporting all that? I don't think that should be necessary
> unless you subshell somewhere.
technically, the "export" can probably be removed. it does help to clearly
mark the variables that are "stateful" across multiple/different calls, but
that's probably also done via the "CDROM_XXX" naming. i'm indifferent.
> And then you can just refer to ${CDROM_CHECK[$i]} rather than using all
> those ugly ${!...}.
yes, i wrote this before i had a grasp of bash arrays. however, for Ulrich's
work, he should just be relocating code without modifying. if we want to do
cleanups, it'd be a follow up. i'm sure i could rewrite quite a bit now just
by reading it.
> > # now we see if the user gave use CD_ROOT ...
> > # if they did, let's just believe them that it's correct
> > if [[ -n ${CD_ROOT}${CD_ROOT_1} ]] ; then
> > local var=
> > cdcnt=0
> > while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
> > ((++cdcnt))
> > var="CD_ROOT_${cdcnt}"
> > [[ -z ${!var} ]] && var="CD_ROOT"
> > if [[ -z ${!var} ]] ; then
> > eerror "You must either use just the
> > CD_ROOT" eerror "or specify ALL the CD_ROOT_X variables."
> > eerror "In this case, you will need" \
> > "${CDROM_TOTAL_CDS} CD_ROOT_X
> > variables." die "could not locate CD_ROOT_${cdcnt}"
> > fi
> > done
> > export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
> > einfo "Found CD #${CDROM_CURRENT_CD} root at
> > ${CDROM_ROOT}"
>
> #1. You are using 1 in variable calls, I don't see a reason to use
> longer ${CDROM_CURRENT_CD} in this one output case.
i don't know what you mean
-mike
signature.asc
Description: This is a digitally signed message part.
