Re: [PATCH v3 11/23] log_ref_setup(): separate code for create vs non-create

On Sat, Dec 31, 2016 at 04:12:51AM +0100, Michael Haggerty wrote:

+	} else {
+		logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);
 		if (logfd < 0) {
-			strbuf_addf(err, "unable to append to '%s': %s",
-				    logfile->buf, strerror(errno));
-			return -1;
+			if (errno == ENOENT || errno == EISDIR) {
+				/*
+				 * The logfile doesn't already exist,
+				 * but that is not an error; it only
+				 * means that we won't write log
+				 * entries to it.
+				 */
+			} else {
+				strbuf_addf(err, "unable to append to '%s': %s",
+					    logfile->buf, strerror(errno));
+				return -1;
+			}
 		}
 	}
 
-	adjust_shared_perm(logfile->buf);
-	close(logfd);
+	if (logfd >= 0) {
+		adjust_shared_perm(logfile->buf);
+		close(logfd);
+	}
+

Hmm. I would have thought in the existing-logfile case that we would not
need to adjust_shared_perm(). But maybe we just do it anyway to pick up
potentially-changed config.

I also had to double-take at this close(). Aren't we calling this
function so we can actually write to the log? But I skipped ahead in
your series and see you address that confusion. :)

-Peff