[lm-sensors] [PATCH] bug in sensors.d reading: not all filesystems

[lm-sensors] [PATCH] bug in sensors.d reading: not all filesystems

[Yuriy Kaminskiy]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

Hello!
  Most filesystems (in particular - reiserfs, jfs, xfs, nfs); maybe,
some legacy installation of ext2/3 too) always return dirent->dt_type =
DT_UNKNOWN, so libsensors fails to parse /etc/sensors.d.
  Also, symlink (DT_LNK) can point to directory, or FIFO/socket/device
(very bad!) - current check insufficient anyways.
  I've attached [somewhat hackerish] patch to solve both problem.
=== cut !man 3 readdir ===
Currently,  only  some  file systems (among them: ext2, etx3, and ext4)
have full support returning the file type in d_type.  All  applications
must properly handle a return of DT_UNKNOWN.
=== cut ===
  Maybe, it would be better to remove dt_type check and use stat() always.

[-- Attachment #2: lm_sensors-3.1.0-dt_unknown-fix-3.patch --]
[-- Type: text/x-diff, Size: 1545 bytes --]

Index: lm_sensors-3.1.0/lib/init.c
===================================================================
--- lm_sensors-3.1.0/lib/init.c.orig	2009-06-01 05:10:42.000000000 +0400
+++ lm_sensors-3.1.0/lib/init.c	2009-06-05 03:17:49.000000000 +0400
@@ -29,6 +29,7 @@
 #include <string.h>
 #include <errno.h>
 #include <dirent.h>
+#include <sys/stat.h>
 #include "sensors.h"
 #include "data.h"
 #include "error.h"
@@ -118,9 +119,10 @@ exit_cleanup:
 	return err;
 }
 
+static int need_check_stat;
 static int config_file_filter(const struct dirent *entry)
 {
-	return (entry->d_type == DT_REG || entry->d_type == DT_LNK)
+	return (entry->d_type == DT_REG || (need_check_stat |= entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN))
 	    && entry->d_name[0] != '.';		/* Skip hidden files */
 }
 
@@ -129,6 +131,7 @@ static int add_config_from_dir(const cha
 	int count, res, i;
 	struct dirent **namelist;
 
+	need_check_stat = 0;
 	count = scandir(dir, &namelist, config_file_filter, alphasort);
 	if (count < 0) {
 		/* Do not return an error if directory does not exist */
@@ -143,6 +146,7 @@ static int add_config_from_dir(const cha
 		int len;
 		char path[PATH_MAX];
 		FILE *input;
+		struct stat st;
 
 		len = snprintf(path, sizeof(path), "%s/%s", dir,
 			       namelist[i]->d_name);
@@ -151,6 +155,9 @@ static int add_config_from_dir(const cha
 			continue;
 		}
 
+		if (need_check_stat && (stat(path, &st) < 0 || !S_ISREG(st.st_mode)))
+			continue;
+
 		input = fopen(path, "r");
 		if (input) {
 			res = parse_config(input, path);

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] bug in sensors.d reading: not all

[Jean Delvare]

Hi Yuriy,

On Fri, 05 Jun 2009 03:25:06 +0400, Yuriy Kaminskiy wrote:
> Hello!
>   Most filesystems (in particular - reiserfs, jfs, xfs, nfs); maybe,
> some legacy installation of ext2/3 too) always return dirent->dt_type > DT_UNKNOWN, so libsensors fails to parse /etc/sensors.d.
>   Also, symlink (DT_LNK) can point to directory, or FIFO/socket/device
> (very bad!) - current check insufficient anyways.
>   I've attached [somewhat hackerish] patch to solve both problem.
> == cut !man 3 readdir => Currently,  only  some  file systems (among them: ext2, etx3, and ext4)
> have full support returning the file type in d_type.  All  applications
> must properly handle a return of DT_UNKNOWN.
> == cut =
Thanks for reporting, I had totally missed the fact that not all
filesystems implemented this interface.

>   Maybe, it would be better to remove dt_type check and use stat() always.

Yes, I think so. I don't much like the complexity of your proposed
solution. The problem is simple, it should have a simple solution.
Performance isn't critical for this part of the code, what matters is
robustness and readability.

What about the simplified version below?

Index: lib/init.c
=================================--- lib/init.c	(révision 5729)
+++ lib/init.c	(copie de travail)
@@ -23,12 +23,14 @@
 #define _BSD_SOURCE
 
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <locale.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
 #include <errno.h>
 #include <dirent.h>
+#include <unistd.h>
 #include "sensors.h"
 #include "data.h"
 #include "error.h"
@@ -120,8 +122,7 @@
 
 static int config_file_filter(const struct dirent *entry)
 {
-	return (entry->d_type = DT_REG || entry->d_type = DT_LNK)
-	    && entry->d_name[0] != '.';		/* Skip hidden files */
+	return entry->d_name[0] != '.';		/* Skip hidden files */
 }
 
 static int add_config_from_dir(const char *dir)
@@ -143,6 +144,7 @@
 		int len;
 		char path[PATH_MAX];
 		FILE *input;
+		struct stat st;
 
 		len = snprintf(path, sizeof(path), "%s/%s", dir,
 			       namelist[i]->d_name);
@@ -151,6 +153,10 @@
 			continue;
 		}
 
+		/* Only accept regular files */
+		if (stat(path, &st) < 0 || !S_ISREG(st.st_mode))
+			continue;
+
 		input = fopen(path, "r");
 		if (input) {
 			res = parse_config(input, path);


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] bug in sensors.d reading: not all

[Yuriy Kaminskiy]

Jean Delvare wrote:
> Performance isn't critical for this part of the code, what matters is
> robustness and readability.
> 
> What about the simplified version below?
Works fine (lm_sensors 3.1.0-2 from debian sid + this patch, reiserfs root).

Tested-by: Yuriy Kaminskiy <yumkam-at-mail.ru>


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] bug in sensors.d reading: not all

[Jean Delvare]

On Fri, 05 Jun 2009 18:30:52 +0400, Yuriy Kaminskiy wrote:
> Jean Delvare wrote:
> > Performance isn't critical for this part of the code, what matters is
> > robustness and readability.
> > 
> > What about the simplified version below?
> Works fine (lm_sensors 3.1.0-2 from debian sid + this patch, reiserfs root).
> 
> Tested-by: Yuriy Kaminskiy <yumkam-at-mail.ru>

Thanks for the quick test. I've committed the patch to SVN, and added
it to the list of recommended patches. Probably we'll have a new
release soon.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors