[patch] various bugs and strncpy abuse followup

[Tito]

On Wednesday 7 June 2006 00:34, Erik Hovland wrote:
> After the very helpful feedback and the continual searching for bugs, I
> have expanded my patch associated to various bugs all over busybox.
> 
> This patchset has gotten bigger. So I have taken to splitting them up by
> file and annotating them in patch. I hope that is useful. Let me know if
> there is some way I can make this easier to go over or help with
> acceptance.
> 
> Feel free to address each patch seperately in responding. I would not be
> surprised at all if I accidentally introduced regressions. I have not
> yet tried running the unit testing. But I have attempted to understand
> why each bug is a bug and addressed it according to its severity.
> 
> I appreciate any attention you might give this work.
> 

Hi,
some comments about the devfsd patch:

1)

--- miscutils/devfsd.c	(revision 15298)
+++ miscutils/devfsd.c	(working copy)
@@ -964,8 +964,9 @@
 				regexpr, numexpr);
 
 	if ( !make_dir_tree (destination) || lstat (source, &source_stat) != 0)
-			return;
-	lstat (destination, &dest_stat);
+		return;
+	if (lstat (destination, &dest_stat) != 0)
+		return;;
 	new_mode = source_stat.st_mode & ~S_ISVTX;
 	if (info->type == DEVFSD_NOTIFY_CREATE)
 		new_mode |= S_ISVTX;

This is unneeded. The original code does:

   if (lstat (destination, &dest_stat) != 0) dest_stat.st_mode = 0;

but we have already initialised dest_stat.st_mode to 0 so we can spare the if statement.
Please drop this part.

2) 

@@ -1090,7 +1091,8 @@
 
 	dest_stat.st_mode = 0;
 	snprintf (dpath, sizeof dpath, "%s%s", mount_point, spath + rootlen);
-	lstat (dpath, &dest_stat);
+	if (lstat (dpath, &dest_stat) < 0)
+		bb_error_msg("Unable to stat %s", dpath);
 
 	if ( S_ISLNK (source_stat.st_mode) || (source_stat.st_mode & S_ISVTX) )
 		copy_inode (dpath, &dest_stat, (source_stat.st_mode & ~S_ISVTX) , spath, &source_stat);

Originally was:

	}
	snprintf (dpath, sizeof dpath, "%s%s", mount_point, spath + rootlen);
	if (lstat (dpath, &dest_stat) != 0) dest_stat.st_mode = 0;

Same as above. Please drop this part.

3)
@@ -1311,9 +1313,9 @@
 	/* compare_string_array returns i>=0  */
 	i=compare_string_array(field_names, variable);

Good catch, at the time I wrote this compare_string_array didn't return values < 0.
BTW, if I remember correctly here it is unlikely to happen, but who knows.....
Maybe we should fix also the comment or remove it totally.

-	if ( i > 6 && (i > 1 && gv_info == NULL))
-			return (NULL);
-	if( i >= 0 || i <= 3)
+	if (i > 6 || (i > 1 && gv_info == NULL) || i < 0)
+		return (NULL);

Good catch too!!!

+	if( i <= 3  && i >= 0 )
 	{
 		debug_msg_logger(LOG_INFO, "%s: i=%d %s", __FUNCTION__, i ,field_names[i+7]);
 		return(field_names[i+7]);


Thanks for auditing this old code.

Ciao,
Tito