[BusyBox] [PATCH] devfsd new patch

Tito farmatito at tiscali.it
Sat Nov 20 14:00:18 UTC 2004


On Saturday 20 November 2004 OO:54, Bastian Blank wrote:
> On Sat, Nov 20, 2004 at 03:03:21AM +0100, Tito wrote:
> > This is strange as the man pages don't report errno setting for fflush, fprintf and vfprintf.
> 
> No, this functions are ANSI-C and don't use errno at all. errno is
> POSIX.
> 
> > The only call in msg_logger() that can change errno seems to be access().
> 
> No, the value of errno is undefined after a library call which may not
> change errno.
> 
> > So if you restore or zero errno after access is called or at the end of the msg_logger your problem
> > should be solved or am i missing something important here?
> > 
> > > msg_logger(), so that errno must be restored before bb_verror_msg() 
> > > is called in order to not generate the false error message.
> 
> It is a bug itself to relay on the value of errno after something else
> than the original call.
Hi,
yes that's what I suspected, now I learned it.

This new patch applies on CVS and should fix the problem reported by Allen Chan
by taking into account Bastian's suggestions.
I tested it in many situations:
with /dev/log 
without /dev/log
with OPTIONAL_INCLUDE file_that_exists
with OPTIONAL_INCLUDE file_that_exists_but_is_zero_size 
with OPTIONAL_INCLUDE file_that_not_exists
with OPTIONAL_INCLUDE dir_that_not_exists
with OPTIONAL_INCLUDE dir_that_exists_but_is_empty    
and it seems to me that it reports errors correctly.

Nov 20 14:46:26 localhost devfsd: Stopping devfsd daemon:  succeeded
Nov 20 14:46:31 localhost devfsd: read config file: /etc/devfs/nothing/: No such file or directory 
Nov 20 14:46:31 localhost devfsd: read config file: /etc/devfs/nothing/nothing: No such file or directory 
( No error for /etc/devfsd.conf )

Busybox config is:

# Miscellaneous Utilities
#
# CONFIG_ADJTIMEX is not set
# CONFIG_CROND is not set
# CONFIG_CRONTAB is not set
# CONFIG_DC is not set
CONFIG_DEVFSD=y
CONFIG_DEVFSD_MODLOAD=y
CONFIG_DEVFSD_FG_NP=y
CONFIG_DEVFSD_VERBOSE=y
# CONFIG_LAST is not set

#
# Debugging Options
#
# CONFIG_DEBUG is not set

BTW there is also a little reduction in size:
root at localhost:/dev/pts/4:/root/Desktop/busybox/miscutils# size devfsd.o
   text    data     bss     dec     hex filename
  10176     392     543   11111    2b67 devfsd.o
root at localhost:/dev/pts/4:/root/Desktop/busybox/miscutils# size devfsd.o.orig
   text    data     bss     dec     hex filename
  10236     392     543   11171    2ba3 devfsd.o.orig

 and some indentation fixes.

Bastian and Allen would you mind to take a look at it and test it?

Ciao,
Tito
-------------- next part --------------
--- miscutils/devfsd_orig.c	2004-11-19 21:30:36.000000000 +0100
+++ miscutils/devfsd.c	2004-11-20 14:23:37.025101368 +0100
@@ -309,16 +309,18 @@
 	va_list ap;
 
 	va_start(ap, fmt);
-	if (access ("/dev/log", F_OK) == 0)
-	{
+	if (access ("/dev/log", F_OK) == 0) {
 		openlog(bb_applet_name, 0, LOG_DAEMON);
 		vsyslog( pri , fmt , ap);
 		closelog();
- 	}
-#ifndef CONFIG_DEBUG
-	else
-#endif
+#ifndef CONFIG_DEBUG	
+	} else {
 		bb_verror_msg(fmt, ap);
+	}
+#else
+	}
+	bb_verror_msg(fmt, ap);
+#endif	
 	va_end(ap);
 	if(die==DIE)
 		exit(EXIT_FAILURE);
@@ -340,26 +342,26 @@
 {
 	switch ( fork () )
 	{
-	case 0:
-		/*  Child  */
-		break;
-	case -1:
-		/*  Parent: Error  : die or return */
+		case 0:
+			/*  Child  */
+			break;
+		case -1:
+			/*  Parent: Error  : die or return */
 #ifdef CONFIG_DEVFSD_VERBOSE
-		msg_logger(die, LOG_ERR,(char *) bb_msg_memory_exhausted);
+			msg_logger(die, LOG_ERR,(char *) bb_msg_memory_exhausted);
 #else
-		if(die == DIE)
-			exit(EXIT_FAILURE);
+			if(die == DIE)
+				exit(EXIT_FAILURE);
 #endif
-		return;
-	default:
-		/*  Parent : ok : return or exit  */
-		if(arg0 != NULL)
-		{
-			wait (NULL);
 			return;
-		}
-		exit (EXIT_SUCCESS);
+		default:
+			/*  Parent : ok : return or exit  */
+			if(arg0 != NULL)
+			{
+				wait (NULL);
+				return;
+			}
+			exit (EXIT_SUCCESS);
 	}
 	 /* Child : if arg0 != NULL do execvp */
 	if(arg0 != NULL )
@@ -566,48 +568,57 @@
 #ifdef CONFIG_DEBUG
 	msg_logger( NO_DIE, LOG_INFO, "read_config_file(): %s\n", path);
 #endif
-	if (stat (path, &statbuf) != 0 || statbuf.st_size == 0 )
-		goto read_config_file_err;
-
-	if ( S_ISDIR (statbuf.st_mode) )
-	{
-		/* strip last / from dirname so we don't need to check for it later */
-		while( path  && path[1]!='\0' && path[strlen(path)-1] == '/')
-			path[strlen(path) -1] = '\0';
-
-		dir_operation(READ_CONFIG, path, 0, event_mask);
-		return;
-	}
-
-	if ( ( fp = fopen (path, "r") ) != NULL )
+	if (stat (path, &statbuf) == 0 )
 	{
-		while (fgets (buf, STRING_LENGTH, fp) != NULL)
+		/* Don't read 0 length files */
+		/*if( statbuf.st_size == 0 )
+			return;*/
+		if ( S_ISDIR (statbuf.st_mode) )
 		{
-			/*  GETS(3)       Linux Programmer's Manual
-			fgets() reads in at most one less than size characters from stream  and
-			stores  them  into  the buffer pointed to by s.  Reading stops after an
-			EOF or a newline.  If a newline is read, it is stored into the  buffer.
-			A '\0' is stored after the last character in the buffer.
-			*/
-			/*buf[strlen (buf) - 1] = '\0';*/
-			/*  Skip whitespace  */
-			for (line = buf; isspace (*line); ++line)
-				/*VOID*/;
-			if (line[0] == '\0' || line[0] == '#' )
-				continue;
-			process_config_line (line, event_mask);
+			/* strip last / from dirname so we don't need to check for it later */
+			while( path  && path[1]!='\0' && path[strlen(path)-1] == '/')
+				path[strlen(path) -1] = '\0';
+	
+			dir_operation(READ_CONFIG, path, 0, event_mask);
+			return;
+		}
+	
+		if ( ( fp = fopen (path, "r") ) != NULL )
+		{
+			while (fgets (buf, STRING_LENGTH, fp) != NULL)
+			{
+				/*  GETS(3)       Linux Programmer's Manual
+				fgets() reads in at most one less than size characters from stream  and
+				stores  them  into  the buffer pointed to by s.  Reading stops after an
+				EOF or a newline.  If a newline is read, it is stored into the  buffer.
+				A '\0' is stored after the last character in the buffer.
+				*/
+				/*buf[strlen (buf) - 1] = '\0';*/
+				/*  Skip whitespace  */
+				for (line = buf; isspace (*line); ++line)
+					/*VOID*/;
+				if (line[0] == '\0' || line[0] == '#' )
+					continue;
+				process_config_line (line, event_mask);
+			}
+			fclose (fp);
+			/*errno=0;*/
+		}
+		else
+		{
+			goto read_config_file_err;
 		}
-		fclose (fp);
-		errno=0;
 	}
+	else
+	{
 read_config_file_err:
 #ifdef CONFIG_DEVFSD_VERBOSE
-	msg_logger(((optional ==  0 ) && (errno == ENOENT))? DIE : NO_DIE, LOG_ERR, "read config file: %s: %m\n", path);
+		msg_logger(((optional ==  0 ) && (errno == ENOENT))? DIE : NO_DIE, LOG_ERR, "read config file: %s: %m\n", path);
 #else
-	if(optional ==  0  && errno == ENOENT)
-		exit(EXIT_FAILURE);
+		if(optional ==  0  && errno == ENOENT)
+			exit(EXIT_FAILURE);
 #endif
-	return;
+	}
 }   /*  End Function read_config_file   */
 
 static void process_config_line (const char *line, unsigned long *event_mask)


More information about the busybox mailing list