Extension of D41893 for directories back to files.
Details
Details
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
I think this part is OK now. That will at least ensure upgrades work, but not rollbacks.
| usr.sbin/freebsd-update/freebsd-update.sh | ||
|---|---|---|
| 2924 | This could be shortened as: if [ -e "${BASEDIR}/${FPATH}" ] && [ ${TYPE} = d ] && ! [ -d "${BASEDIR}/${FPATH}" ]; thensince && short-circuits. But it's mostly a matter of preference. | |
Comment Actions
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.
Comment Actions
This needs to be rebased and reworked after the slightly different approach taken in f6d37c9ca13f8ab0ef32cf5344daecb8122d1e85.
| usr.sbin/freebsd-update/freebsd-update.sh | ||
|---|---|---|
| 2906 | 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 | ||
|---|---|---|
| 2924 | 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 | ||
|---|---|---|
| 2924 | 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 :) | |
Comment Actions
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)
Comment Actions
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. | |