[3048] in BarnOwl Developers
Re: [PATCH] owl_util_file_deleteline should not error on non-existent file
daemon@ATHENA.MIT.EDU (Jason Gross)
Tue May 17 12:15:18 2011
In-Reply-To: <alpine.DEB.1.10.1105171201030.30967@dr-wily.mit.edu>
From: Jason Gross <jgross@MIT.EDU>
Date: Tue, 17 May 2011 12:14:43 -0400
To: David A Benjamin <davidben@mit.edu>
Cc: barnowl-dev@mit.edu
--001e680f1400b95cba04a37b1362
Content-Type: text/plain; charset=ISO-8859-1
We don't want to display an error at all on the startup file not existing
when we call startup, so setting errno carefully won't work.
If we worry about race conditions (which I'm not sure we should), then I
don't see how you can check before owl_util_file_deleteline. Should we
worry about race conditions?
Alternatively, I could make owl_util_file_deleteline take an optional
boolean error_on_enoent.
-Jason
On Tue, May 17, 2011 at 12:04 PM, David A Benjamin <davidben@mit.edu> wrote:
> I think this is the wrong place to add the check. If I ask you to delete a
> line from a file, and you find that the file doesn't exist, that is
> definitely grounds for a failure.
>
> Better would be to either make owl_util_file_deleteline report what kind of
> error (errno should work here, but you have to be careful to re-set it
> because owl_function_error may set errno) so the caller can interpret it
> correctly or to have startup check before calling owl_util_file_deleteline.
>
> David
>
>
> On Tue, 17 May 2011, Jason Gross wrote:
>
> This fixes trac #175: '"startup" command produces error when run for the
>> first time'. The startup command deletes duplicate lines, and thus
>> errored when no file existed. The purpose of owl_util_file_deleteline
>> is to make sure that the given line is no longer in the file, so there
>> seems to be no harm in returning 0 (meaning no lines were deleted) on
>> the file not existing.
>> ---
>> util.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/util.c b/util.c
>> index 82f36ef..44899f0 100644
>> --- a/util.c
>> +++ b/util.c
>> @@ -423,6 +423,7 @@ gchar *owl_util_recursive_resolve_link(const char
>> *filename)
>> * case-insensitive.
>> *
>> * Returns the number of lines removed on success. Returns -1 on failure.
>> + * Returns 0 if the file does not exist; the line wasn't present.
>> */
>> int owl_util_file_deleteline(const char *filename, const char *line, int
>> backup)
>> {
>> @@ -433,6 +434,7 @@ int owl_util_file_deleteline(const char *filename,
>> const char *line, int backup)
>> int numremoved = 0;
>>
>> if ((old = fopen(filename, "r")) == NULL) {
>> + if (errno == ENOENT) return 0;
>> owl_function_error("Cannot open %s (for reading): %s",
>> filename, strerror(errno));
>> return -1;
>> --
>> 1.7.2.3
>>
>>
>>
--001e680f1400b95cba04a37b1362
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
<div>We don't want to display an error at all on the startup file not e=
xisting when we call startup, so setting errno carefully won't work.</d=
iv><div><br></div>If we worry about race conditions (which I'm not sure=
we should), then I don't see how you can check before owl_util_file_de=
leteline. =A0Should we worry about race conditions?<div>
Alternatively, I could make=A0owl_util_file_deleteline take an optional boo=
lean error_on_enoent.</div><div><br></div><div>-Jason<br><div><br><div clas=
s=3D"gmail_quote">On Tue, May 17, 2011 at 12:04 PM, David A Benjamin <span =
dir=3D"ltr"><<a href=3D"mailto:davidben@mit.edu">davidben@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;">I think this is the wrong place to add the =
check. If I ask you to delete a line from a file, and you find that the fil=
e doesn't exist, that is definitely grounds for a failure.<br>
<br>
Better would be to either make owl_util_file_deleteline report what kind of=
error (errno should work here, but you have to be careful to re-set it bec=
ause owl_function_error may set errno) so the caller can interpret it corre=
ctly or to have startup check before calling owl_util_file_deleteline.<br>
<font color=3D"#888888">
<br>
David</font><div><div></div><div class=3D"h5"><br>
<br>
On Tue, 17 May 2011, Jason Gross wrote:<br>
<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
This fixes trac #175: '"startup" command produces error when =
run for the<br>
first time'. =A0The startup command deletes duplicate lines, and thus<b=
r>
errored when no file existed. =A0The purpose of owl_util_file_deleteline<br=
>
is to make sure that the given line is no longer in the file, so there<br>
seems to be no harm in returning 0 (meaning no lines were deleted) on<br>
the file not existing.<br>
---<br>
util.c | =A0 =A02 ++<br>
1 files changed, 2 insertions(+), 0 deletions(-)<br>
<br>
diff --git a/util.c b/util.c<br>
index 82f36ef..44899f0 100644<br>
--- a/util.c<br>
+++ b/util.c<br>
@@ -423,6 +423,7 @@ gchar *owl_util_recursive_resolve_link(const char *file=
name)<br>
=A0* case-insensitive.<br>
=A0*<br>
=A0* Returns the number of lines removed on success. =A0Returns -1 on failu=
re.<br>
+ * Returns 0 if the file does not exist; the line wasn't present.<br>
=A0*/<br>
int owl_util_file_deleteline(const char *filename, const char *line, int ba=
ckup)<br>
{<br>
@@ -433,6 +434,7 @@ int owl_util_file_deleteline(const char *filename, cons=
t char *line, int backup)<br>
=A0int numremoved =3D 0;<br>
<br>
=A0if ((old =3D fopen(filename, "r")) =3D=3D NULL) {<br>
+ =A0 =A0if (errno =3D=3D ENOENT) return 0;<br>
=A0 =A0owl_function_error("Cannot open %s (for reading): %s",<br=
>
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 filename, strerror(errno));<br=
>
=A0 =A0return -1;<br>
-- <br>
1.7.2.3<br>
<br>
<br>
</blockquote>
</div></div></blockquote></div><br></div></div>
--001e680f1400b95cba04a37b1362--
