Extension of D41893 for directories back to files.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I think this part is OK now. That will at least ensure upgrades work, but not rollbacks.
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2918 | This could be shortened as: if [ -e "${BASEDIR}/${FPATH}" ] && [ ${TYPE} = d ] && ! [ -d "${BASEDIR}/${FPATH}" ]; then since && short-circuits. But it's mostly a matter of preference. |
I think this part is OK now. That will at least ensure upgrades work, but not rollbacks.
Oops, I uploaded the wrong diff to this review. D41893 is the review for the upgrade (but not rollback) first part.
This needs to be rebased and reworked after the slightly different approach taken in f6d37c9ca13f8ab0ef32cf5344daecb8122d1e85.
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2900 | Oh I guess -e && -d is redundant, will drop -e |
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2983 | If you are deleting directories in dir_conflict using rm -rf I think it is reasonable to delete these offending files with rm -f too, and in that case it is not necessary to do the if [ -e, since rm will shut up and not fail in such cases. |
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2983 | I've switched this to [ -f "${BASEDIR}/${FPATH} ] and [ -L "${BASEDIR}/${FPATH} ] locally now, because we still get an error message: $ touch not-a-dir |
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2918 | Or maybe even: if [ -e "${BASEDIR}/${FPATH}" -a ${TYPE} = d -a ! -d "${BASEDIR}/${FPATH}" ]; then (it reduces the amount of calls to test through [, although it is a built-in in sh anyway) |
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2918 | That doesn't have the benefit of short-circuiting though (and [ is a built-in anyhow, as you say). In any case that change is already committed and is independent of this review :) |
I also generally avoid that (e.g., with rm -f as already suggested here) because otherwise it creates TOCTOU conditions. (Low risk here but you never know)
rm -f doesn't avoid the error.
$ touch not-a-dir $ rm -f not-a-dir/does-not-exist rm: not-a-dir/does-not-exist: Not a directory
usr.sbin/freebsd-update/freebsd-update.sh | ||
---|---|---|
2983 | True, I did not think of that. |