[2785] in BarnOwl Developers
Re: [PATCH 1/2] Fixed :unsub to make it work if ~/.zephyr.subs is a symlink (trac 133)
daemon@ATHENA.MIT.EDU (Karl Ramm)
Mon Nov 15 16:38:24 2010
From: Karl Ramm <kcr@1ts.org>
To: barnowl-dev@mit.edu
Date: Mon, 15 Nov 2010 16:38:22 -0500
In-Reply-To: <1289760021-4478-2-git-send-email-jgross@mit.edu> (Jason Gross's
message of "Sun, 14 Nov 2010 13:40:20 -0500")
I said looks good to me, before, but I notice that canonicalize_file_name
is a GNUism; do we want to give up and only work on glibc systems?
kcr
(Oh hey, that can has worms crawling out)
Jason Gross <jgross@MIT.EDU> writes:
> From: Jason <jgross@mit.edu>
>
> link() fails on AFS if the target is a symlink. Additionally, on
> systems where link() succeeds, barnowl overwrites the symlink, instead
> of the actual file. To fix this, I've changed owl_util_file_deleteline
> to call canonicalize_file_name on the passed file name first. The new
> (backup) file is written in the resolved directory, not the directory
> where the symlink itself is found.
>
> Additionally, I corrected free(filename) to owl_free(filename) in the
> two places that it occurred.
> ---
> util.c | 34 ++++++++++++++++++++++++----------
> 1 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/util.c b/util.c
> index 6546c73..cdbfd01 100644
> --- a/util.c
> +++ b/util.c
> @@ -384,38 +384,50 @@ char *owl_util_stripnewlines(const char *in)
> */
> int owl_util_file_deleteline(const char *filename, const char *line, int backup)
> {
> - char *backupfile, *newfile, *buf = NULL;
> + char *actual_filename, *backupfile, *newfile, *buf = NULL;
> FILE *old, *new;
> struct stat st;
> int numremoved = 0;
>
> + /* canonicalize filename, so that we deal with with symlinks correctly */
> + actual_filename = canonicalize_file_name(filename);
> + if (actual_filename == NULL) {
> + owl_function_error("Cannot canonicalize file name %s: %s",
> + actual_filename, strerror(errno));
> + return 0;
> + }
> +
> if ((old = fopen(filename, "r")) == NULL) {
> owl_function_error("Cannot open %s (for reading): %s",
> filename, strerror(errno));
> + free(actual_filename);
> return 0;
> }
>
> if (fstat(fileno(old), &st) != 0) {
> owl_function_error("Cannot stat %s: %s", filename, strerror(errno));
> + free(actual_filename);
> return 0;
> }
>
> - newfile = owl_sprintf("%s.new", filename);
> + newfile = owl_sprintf("%s.new", actual_filename);
> if ((new = fopen(newfile, "w")) == NULL) {
> owl_function_error("Cannot open %s (for writing): %s",
> - filename, strerror(errno));
> - free(newfile);
> + actual_filename, strerror(errno));
> + owl_free(newfile);
> fclose(old);
> + free(actual_filename);
> return 0;
> }
>
> if (fchmod(fileno(new), st.st_mode & 0777) != 0) {
> owl_function_error("Cannot set permissions on %s: %s",
> - filename, strerror(errno));
> + actual_filename, strerror(errno));
> unlink(newfile);
> fclose(new);
> - free(newfile);
> + owl_free(newfile);
> fclose(old);
> + free(actual_filename);
> return 0;
> }
>
> @@ -430,9 +442,9 @@ int owl_util_file_deleteline(const char *filename, const char *line, int backup)
> fclose(old);
>
> if (backup) {
> - backupfile = owl_sprintf("%s.backup", filename);
> + backupfile = owl_sprintf("%s.backup", actual_filename);
> unlink(backupfile);
> - if (link(filename, backupfile) != 0) {
> + if (link(actual_filename, backupfile) != 0) {
> owl_function_error("Cannot link %s: %s", backupfile, strerror(errno));
> owl_free(backupfile);
> unlink(newfile);
> @@ -442,15 +454,17 @@ int owl_util_file_deleteline(const char *filename, const char *line, int backup)
> owl_free(backupfile);
> }
>
> - if (rename(newfile, filename) != 0) {
> + if (rename(newfile, actual_filename) != 0) {
> owl_function_error("Cannot move %s to %s: %s",
> - newfile, filename, strerror(errno));
> + newfile, actual_filename, strerror(errno));
> numremoved = 0;
> }
>
> unlink(newfile);
> owl_free(newfile);
>
> + free(actual_filename);
> +
> return numremoved;
> }
