[2777] in BarnOwl Developers
Re: [PATCH 1/2] Fixed :unsub to make it work if ~/.zephyr.subs is a
daemon@ATHENA.MIT.EDU (Jason Gross)
Sun Nov 14 03:51:42 2010
In-Reply-To: <1289708556-7552-1-git-send-email-jgross@mit.edu>
From: Jason Gross <jgross@MIT.EDU>
Date: Sun, 14 Nov 2010 03:51:08 -0500
To: barnowl-dev@mit.edu
--000e0cd6aca68a23360494ff6e89
Content-Type: text/plain; charset=ISO-8859-1
I misread the description/comment of the ticked, and thus mistakenly
concluded that the error was on rename. It was actually trying to hardlink
to a symlink, and, even if this worked, I think the old behavior is wrong
(we don't want to rewrite the symlink, we want to rewrite the underlying
file). Sorry about that. The new extended description of my commit should
be:
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.
On Sat, Nov 13, 2010 at 11:22 PM, Jason Gross <jgross@mit.edu> wrote:
> From: Jason <jgross@mit.edu>
>
> rename() fails if the new name is a symlink (despite the man page for
> rename saying that it should overwrite the file!?). 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 | 32 ++++++++++++++++++++++----------
> 1 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/util.c b/util.c
> index 6546c73..e00da2e 100644
> --- a/util.c
> +++ b/util.c
> @@ -384,7 +384,7 @@ 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;
> @@ -400,22 +400,32 @@ int owl_util_file_deleteline(const char *filename,
> const char *line, int backup)
> return 0;
> }
>
> - newfile = owl_sprintf("%s.new", filename);
> + /* canonicalize filename and handle errors */
> + 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;
> + }
> +
> + 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 +440,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 +452,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;
> }
>
> --
> 1.7.1
>
>
--000e0cd6aca68a23360494ff6e89
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
I misread the description/comment of the ticked, and thus mistakenly conclu=
ded that the error was on rename. =A0It was actually trying to hardlink to =
a symlink, and, even if this worked, I think the old behavior is wrong (we =
don't want to rewrite the symlink, we want to rewrite the underlying fi=
le). =A0Sorry about that. =A0The new extended description of my commit shou=
ld be:<div>
<br></div><div><div>link() fails on AFS if the target is a symlink. =A0Addi=
tionally, on</div><div>systems where link() succeeds, barnowl overwrites th=
e symlink, instead</div><div>of the actual file. =A0To fix this, I've c=
hanged owl_util_file_deleteline</div>
<div>to call canonicalize_file_name on the passed file name first. =A0The n=
ew</div><div>(backup) file is written in the resolved directory, not the di=
rectory</div><div>where the symlink itself is found.</div><div><br></div>
<div>Additionally, I corrected free(filename) to owl_free(filename) in the<=
/div><div>two places that it occurred.</div></div><div><br><div class=3D"gm=
ail_quote">On Sat, Nov 13, 2010 at 11:22 PM, Jason Gross <span dir=3D"ltr">=
<<a href=3D"mailto:jgross@mit.edu">jgross@mit.edu</a>></span> wrote:<=
br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex;">From: Jason <<a href=3D"mailto:jgross@mi=
t.edu">jgross@mit.edu</a>><br>
<br>
rename() fails if the new name is a symlink (despite the man page for<br>
rename saying that it should overwrite the file!?). =A0I've changed<br>
owl_util_file_deleteline to call canonicalize_file_name on the passed<br>
file name first. =A0The new (backup) file is written in the resolved<br>
directory, not the directory where the symlink itself is found.<br>
<br>
Additionally, I corrected free(filename) to owl_free(filename) in the<br>
two places that it occurred.<br>
---<br>
=A0util.c | =A0 32 ++++++++++++++++++++++----------<br>
=A01 files changed, 22 insertions(+), 10 deletions(-)<br>
<br>
diff --git a/util.c b/util.c<br>
index 6546c73..e00da2e 100644<br>
--- a/util.c<br>
+++ b/util.c<br>
@@ -384,7 +384,7 @@ char *owl_util_stripnewlines(const char *in)<br>
=A0*/<br>
=A0int owl_util_file_deleteline(const char *filename, const char *line, int=
backup)<br>
=A0{<br>
- =A0char *backupfile, *newfile, *buf =3D NULL;<br>
+ =A0char *actual_filename, *backupfile, *newfile, *buf =3D NULL;<br>
=A0 FILE *old, *new;<br>
=A0 struct stat st;<br>
=A0 int numremoved =3D 0;<br>
@@ -400,22 +400,32 @@ int owl_util_file_deleteline(const char *filename, co=
nst char *line, int backup)<br>
=A0 =A0 return 0;<br>
=A0 }<br>
<br>
- =A0newfile =3D owl_sprintf("%s.new", filename);<br>
+ =A0/* canonicalize filename and handle errors */<br>
+ =A0actual_filename =3D canonicalize_file_name(filename);<br>
+ =A0if (actual_filename =3D=3D NULL) {<br>
+ =A0 =A0owl_function_error("Cannot canonicalize file name %s: %s"=
;,<br>
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0actual_filename, strerror(errn=
o));<br>
+ =A0 =A0return 0;<br>
+ =A0}<br>
+<br>
+ =A0newfile =3D owl_sprintf("%s.new", actual_filename);<br>
=A0 if ((new =3D fopen(newfile, "w")) =3D=3D NULL) {<br>
=A0 =A0 owl_function_error("Cannot open %s (for writing): %s",<b=
r>
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0filename, strerror(errno));<br=
>
- =A0 =A0free(newfile);<br>
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0actual_filename, strerror(errn=
o));<br>
+ =A0 =A0owl_free(newfile);<br>
=A0 =A0 fclose(old);<br>
+ =A0 =A0free(actual_filename);<br>
=A0 =A0 return 0;<br>
=A0 }<br>
<br>
=A0 if (fchmod(fileno(new), st.st_mode & 0777) !=3D 0) {<br>
=A0 =A0 owl_function_error("Cannot set permissions on %s: %s",<b=
r>
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0filename, strerror(errno));<br=
>
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0actual_filename, strerror(errn=
o));<br>
=A0 =A0 unlink(newfile);<br>
=A0 =A0 fclose(new);<br>
- =A0 =A0free(newfile);<br>
+ =A0 =A0owl_free(newfile);<br>
=A0 =A0 fclose(old);<br>
+ =A0 =A0free(actual_filename);<br>
=A0 =A0 return 0;<br>
=A0 }<br>
<br>
@@ -430,9 +440,9 @@ int owl_util_file_deleteline(const char *filename, cons=
t char *line, int backup)<br>
=A0 fclose(old);<br>
<br>
=A0 if (backup) {<br>
- =A0 =A0backupfile =3D owl_sprintf("%s.backup", filename);<br>
+ =A0 =A0backupfile =3D owl_sprintf("%s.backup", actual_filename)=
;<br>
=A0 =A0 unlink(backupfile);<br>
- =A0 =A0if (link(filename, backupfile) !=3D 0) {<br>
+ =A0 =A0if (link(actual_filename, backupfile) !=3D 0) {<br>
=A0 =A0 =A0 owl_function_error("Cannot link %s: %s", backupfile,=
strerror(errno));<br>
=A0 =A0 =A0 owl_free(backupfile);<br>
=A0 =A0 =A0 unlink(newfile);<br>
@@ -442,15 +452,17 @@ int owl_util_file_deleteline(const char *filename, co=
nst char *line, int backup)<br>
=A0 =A0 owl_free(backupfile);<br>
=A0 }<br>
<br>
- =A0if (rename(newfile, filename) !=3D 0) {<br>
+ =A0if (rename(newfile, actual_filename) !=3D 0) {<br>
=A0 =A0 owl_function_error("Cannot move %s to %s: %s",<br>
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0newfile, filename, strerror(er=
rno));<br>
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0newfile, actual_filename, stre=
rror(errno));<br>
=A0 =A0 numremoved =3D 0;<br>
=A0 }<br>
<br>
=A0 unlink(newfile);<br>
=A0 owl_free(newfile);<br>
<br>
+ =A0free(actual_filename);<br>
+<br>
=A0 return numremoved;<br>
=A0}<br>
<font color=3D"#888888"><br>
--<br>
1.7.1<br>
<br>
</font></blockquote></div><br></div>
--000e0cd6aca68a23360494ff6e89--
