thanks to both for comments.
> Date: Fri, 7 Sep 2012 14:02:39 +0200
> From: jhro...@redhat.com
> To: sssd-devel@lists.fedorahosted.org
> Subject: Re: [SSSD] [PATCH] 1371-Missing resolv.conf should be non-fatal
>
> On Fri, Sep 07, 2012 at 12:41:30PM +0200, Michal Židek wrote:
> > On 09/07/2012 05:53 AM, Ariel Barria wrote:
> > >Patch sent for review.
> > >
> > >https://fedorahosted.org/sssd/ticket/1371
> > >
> > >
> > >_______________________________________________
> > >sssd-devel mailing list
> > >sssd-devel@lists.fedorahosted.org
> > >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > Hi Ariel,
> >
> > I have not tested your patch, but I see some problems in it.
> > Do not set errno variable explicitly to 0. It is not how it is ment
> > to be used.
>
> Actually, it might be beneficial to clear errno in some cases. stat(2)
> is probably not one of them as it only sets errno in an error condition.
>
> Here is a nice summary on the errno checking:
> https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=6619179
>
> > Also ENOENT is not the only possible error, we
> > should check for the other errors as well. Also do not clear
> > the return value of stat.
> >
> > The check should look something like:
> >
> > ret = stat( ... );
> > if (ret != 0) {
>
> I would also assign the errno value to ret immediatelly to avoid
> rewriting errno with another DEBUG call for instance.
>
> > if (errno == ENOENT) {
> > /* resolv.conf not found. This is the reason for this patch ...*/;
> > } else {
> > /* ... but we must check for other errors as well
> > (the error msg here can be more general) */
> > }
> > } else {
> > /* Here is everything OK */
> > }
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From b7c2c548f3d6d172bd77293dfaa4d85e2f76b959 Mon Sep 17 00:00:00 2001
From: Ariel Barria <ari...@fedoraproject.org>
Date: Thu, 6 Sep 2012 22:45:12 -0500
Subject: [PATCH] 1371-Missing resolv.conf should be non-fatal
---
src/monitor/monitor.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index d9e84c440d794d2000c3ccb51e3ad02bb3d055f7..213918f9c3696052ebd108bc5cd87c0f0bebb679 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -1918,6 +1918,7 @@ int monitor_process_init(struct mt_ctx *ctx,
int num_providers;
int ret;
int error;
+ struct stat file_stat;
/* Set up the environment variable for the Kerberos Replay Cache */
ret = confdb_get_string(ctx->cdb, ctx,
@@ -1998,8 +1999,25 @@ int monitor_process_init(struct mt_ctx *ctx,
}
#endif
/* Watch for changes to the DNS resolv.conf */
- ret = monitor_config_file(ctx, ctx, RESOLV_CONF_PATH,
- monitor_update_resolv);
+ errno = 0;
+ ret = stat(RESOLV_CONF_PATH, &file_stat);
+
+ if (ret < 0) {
+ ret = errno;
+ if (ret == ENOENT) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("file [%s] is missing, but will skip this check.\n",
+ RESOLV_CONF_PATH));
+ } else {
+ DEBUG(SSSDBG_MINOR_FAILURE,("Could not stat file [%s]. Error [%d:%s]\n",
+ RESOLV_CONF_PATH, ret, strerror(ret)));
+ }
+ ret = 0;
+ } else {
+ ret = monitor_config_file(ctx, ctx, RESOLV_CONF_PATH,
+ monitor_update_resolv);
+ }
+
if (ret != EOK) {
return ret;
}
--
1.7.11.2
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
