[3049] in BarnOwl Developers
Re: [PATCH] owl_util_file_deleteline should not error on non-existent
daemon@ATHENA.MIT.EDU (David A Benjamin)
Tue May 17 12:21:03 2011
Date: Tue, 17 May 2011 12:21:00 -0400 (EDT)
From: David A Benjamin <davidben@MIT.EDU>
To: Jason Gross <jgross@mit.edu>
cc: barnowl-dev@mit.edu
In-Reply-To: <BANLkTik_7H=m0AvFP+_rtgT+WQ0-Nzonhg@mail.gmail.com>
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
---1257051904-1006294497-1305649260=:30967
Content-Type: TEXT/PLAIN; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
Can you move the error message to the callers, perhaps?
(We really need a more coherant way for passing errors up. Glib has this=20
strange GError thing which... works but is probably a bit much.)
I think it's also fine to not care about the race. It's pretty harmless.
David
On Tue, 17 May 2011, Jason Gross wrote:
> 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.
>=20
> 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. =A0Should we w=
orry about race conditions? Alternatively, I could
> make=A0owl_util_file_deleteline take an optional boolean error_on_enoent.
>=20
> -Jason
>=20
> On Tue, May 17, 2011 at 12:04 PM, David A Benjamin <davidben@mit.edu> wro=
te:
> I think this is the wrong place to add the check. If I ask you to d=
elete 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-se=
t it because owl_function_error may set errno) so
> the caller can interpret it correctly or to have startup check befo=
re calling owl_util_file_deleteline.
>
> David
>=20
>=20
> On Tue, 17 May 2011, Jason Gross wrote:
>
> This fixes trac #175: '"startup" command produces error when run fo=
r the
> first time'. =A0The startup command deletes duplicate lines, and th=
us
> errored when no file existed. =A0The purpose of owl_util_file_delet=
eline
> is to make sure that the given line is no longer in the file, so th=
ere
> seems to be no harm in returning 0 (meaning no lines were deleted) =
on
> the file not existing.
> ---
> util.c | =A0 =A02 ++
> 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 ch=
ar *filename)
> =A0* case-insensitive.
> =A0*
> =A0* Returns the number of lines removed on success. =A0Returns -1 =
on failure.
> + * Returns 0 if the file does not exist; the line wasn't present.
> =A0*/
> int owl_util_file_deleteline(const char *filename, const char *line=
, int backup)
> {
> @@ -433,6 +434,7 @@ int owl_util_file_deleteline(const char *filena=
me, const char *line, int backup)
> =A0int numremoved =3D 0;
>
> =A0if ((old =3D fopen(filename, "r")) =3D=3D NULL) {
> + =A0 =A0if (errno =3D=3D ENOENT) return 0;
> =A0 =A0owl_function_error("Cannot open %s (for reading): %s",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 filename, strerror(errn=
o));
> =A0 =A0return -1;
> --
> 1.7.2.3
>=20
>=20
>=20
>=20
>
---1257051904-1006294497-1305649260=:30967--
