(RT #3666) named could fail to rotate long log files.

developer: marka
reviewer: jinmei

lib/isc/include/isc/log.h

@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: log.h,v 1.45 2002/09/09 20:02:22 explorer Exp $ */
+/* $Id: log.h,v 1.46 2002/10/16 13:15:30 jinmei Exp $ */
 
 #ifndef ISC_LOG_H
 #define ISC_LOG_H 1
@@ -61,6 +61,7 @@
 #define ISC_LOG_PRINTTAG	0x0010
 #define ISC_LOG_PRINTALL	0x001F
 #define ISC_LOG_DEBUGONLY	0x1000
+#define ISC_LOG_OPENERR		0x8000		/* internal */
 
 /*
  * Other options.

lib/isc/log.c

@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: log.c,v 1.78 2002/09/09 20:02:21 explorer Exp $ */
+/* $Id: log.c,v 1.79 2002/10/16 13:15:29 jinmei Exp $ */
 
 /* Principal Authors: DCL */
 
@@ -1188,9 +1188,9 @@ greatest_version(isc_logchannel_t *channel, int *greatestp) {
 
 static isc_result_t
 roll_log(isc_logchannel_t *channel) {
-	int i, greatest, digits = 0;
+	int i, n, greatest;
-	char current[FILENAME_MAX + 1];
+	char current[PATH_MAX + 1];
-	char new[FILENAME_MAX + 1];
+	char new[PATH_MAX + 1];
 	const char *path;
 	isc_result_t result;
 
@@ -1234,37 +1234,58 @@ roll_log(isc_logchannel_t *channel) {
 		 * Remove any excess logs on the way to that value.
 		 */
 		while (--greatest >= FILE_VERSIONS(channel)) {
-			sprintf(current, "%s.%d", path, greatest);
+			n = snprintf(current, sizeof(current), "%s.%d",
-			(void)remove(current);
+				     path, greatest);
+			if (n >= (int)sizeof(current) || n < 0)
+				result = ISC_R_NOSPACE;
+			else
+				result = isc_file_remove(current);
+			if (result != ISC_R_SUCCESS &&
+			    result != ISC_R_FILENOTFOUND)
+				syslog(LOG_ERR,
+				       "unable to remove log file '%s.%d': %s",
+				       path, greatest,
+				       isc_result_totext(result));
 		}
 
-	for (i = greatest; i > 0; i /= 10)
-		digits++;
-
-	/*
-	 * Ensure the name fits in the filesystem.  Note that in this will not
-	 * trigger failure until there is going to be a log rolled into a name
-	 * that is too long, not when the maximum possible version name would
-	 * be too long.  Imagine a case where the name for logs 0-9 is exactly
-	 * as long as the maximum filename, but FILE_VERSIONS is configured as
-	 * 11.  log.10's name will be too long, but no error will be triggered
-	 * until log.9 exists and needs to be rolled.
-	 */
-	if (strlen(path) + 1 + digits > FILENAME_MAX)
-		return (ISC_R_INVALIDFILE);
-
 	for (i = greatest; i > 0; i--) {
-		sprintf(current, "%s.%d", path, i - 1);
+		result = ISC_R_SUCCESS;
-		sprintf(new, "%s.%d", path, i);
+		n = snprintf(current, sizeof(current), "%s.%d", path, i - 1);
-		(void)isc_file_rename(current, new);
+		if (n >= (int)sizeof(current) || n < 0)
+			result = ISC_R_NOSPACE;
+		if (result == ISC_R_SUCCESS) {
+			n = snprintf(new, sizeof(new), "%s.%d", path, i);
+			if (n >= (int)sizeof(new) || n < 0)
+				result = ISC_R_NOSPACE;
+		}
+		if (result == ISC_R_SUCCESS)
+			result = isc_file_rename(current, new);
+		if (result != ISC_R_SUCCESS &&
+		    result != ISC_R_FILENOTFOUND)
+			syslog(LOG_ERR,
+			       "unable to rename log file '%s.%d' to "
+			       "'%s.%d': %s", path, i - 1, path, i,
+			       isc_result_totext(result));
 	}
 
 	if (FILE_VERSIONS(channel) != 0) {
-		sprintf(new, "%s.0", path);
+		n = snprintf(new, sizeof(new), "%s.0", path);
-		(void)isc_file_rename(path, new);
+		if (n >= (int)sizeof(new) || n < 0)
-
+			result = ISC_R_NOSPACE;
-	} else if (FILE_VERSIONS(channel) == 0)
+		else
-		(void)remove(path);
+			result = isc_file_rename(path, new);
+		if (result != ISC_R_SUCCESS &&
+		    result != ISC_R_FILENOTFOUND)
+			syslog(LOG_ERR,
+			       "unable to rename log file '%s' to '%s.0': %s",
+			       path, path, isc_result_totext(result));
+	} else {
+		result = isc_file_remove(path);
+		if (result != ISC_R_SUCCESS &&
+		    result != ISC_R_FILENOTFOUND)
+			syslog(LOG_ERR, "unable to remove log file '%s': %s",
+			       path, isc_result_totext(result));
+	}
 
 	return (ISC_R_SUCCESS);
 }
@@ -1290,8 +1311,7 @@ isc_log_open(isc_logchannel_t *channel) {
 	 * and either has no size limit or has reached its size limit.
 	 */
 	if (stat(path, &statbuf) == 0) {
-		regular_file = (statbuf.st_mode & S_IFREG) ?
+		regular_file = S_ISREG(statbuf.st_mode) ? ISC_TRUE : ISC_FALSE;
-							ISC_TRUE : ISC_FALSE;
 		/* XXXDCL if not regular_file complain? */
 		roll = ISC_TF(regular_file &&
 			      statbuf.st_size >= FILE_MAXSIZE(channel));
@@ -1305,9 +1325,18 @@ isc_log_open(isc_logchannel_t *channel) {
 	 */
 	if (result == ISC_R_SUCCESS && roll) {
 		result = roll_log(channel);
-		if (result != ISC_R_SUCCESS)
+		if (result != ISC_R_SUCCESS) {
+			if ((channel->flags & ISC_LOG_OPENERR) == 0) {
+				syslog(LOG_ERR,
+				       "isc_log_open: roll_log '%s' "
+				       "failed: %s",
+				       FILE_NAME(channel),
+				       isc_result_totext(result));
+				channel->flags |= ISC_LOG_OPENERR;
+			}
 			return (result);
 		}
+	}
 
 	result = isc_stdio_open(path, "a", &FILE_STREAM(channel));
 
@@ -1618,12 +1647,17 @@ isc_log_doit(isc_log_t *lctx, isc_logcategory_t *category,
 
 			if (FILE_STREAM(channel) == NULL) {
 				result = isc_log_open(channel);
+				if (result != ISC_R_SUCCESS &&
+				    (channel->flags & ISC_LOG_OPENERR) == 0) {
+					syslog(LOG_ERR,
+					       "isc_log_open '%s' failed: %s",
+					       FILE_NAME(channel),
+					       isc_result_totext(result));
+					channel->flags |= ISC_LOG_OPENERR;
+				}
 				if (result != ISC_R_SUCCESS)
 					break;
-				/*
+				channel->flags &= ~ISC_LOG_OPENERR;
-				 * Probably something more meaningful should be
-				 * done with an error.
-				 */
 			}
 			/* FALLTHROUGH */