3 files changed, 57 insertions, 26 deletions

diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c
index 6ada2a6..8bff732 100644
--- a/discover/grub2/builtins.c
+++ b/discover/grub2/builtins.c
@@ -124,43 +124,58 @@ static int builtin_search(struct grub2_script *script,
 	return 0;
 }
 
+/* Note that GRUB does not follow symlinks in evaluating all file
+ * tests but -s, unlike below. However, it seems like a bad idea to
+ * emulate GRUB's behavior (e.g., it would take extra work), so we
+ * implement the behavior that coreutils' test binary has. */
 static bool builtin_test_op_file(struct grub2_script *script, char op,
 		const char *file)
 {
 	bool result;
-	int len, rc;
-	char *buf;
+	int rc;
+	struct stat statbuf;
 
-	rc = parser_request_file(script->ctx, script->ctx->device,
-			file, &buf, &len);
+	rc = parser_stat_path(script->ctx, script->ctx->device,
+			file, &statbuf);
 	if (rc)
 		return false;
 
 	switch (op) {
 	case 's':
 		/* -s: return true if file exists and has non-zero size */
-		result = len > 0;
+		result = statbuf.st_size > 0;
 		break;
 	case 'f':
-		/* -f: return true if file exists */
-		result = true;
+		/* -f: return true if file exists and is not a directory. This is
+		 * different than the behavior of "test", but is what GRUB does
+		 * (though note as above that we follow symlinks unlike GRUB). */
+		result = !S_ISDIR(statbuf.st_mode);
 		break;
 	default:
 		result = false;
 
 	}
 
-	talloc_free(buf);
 	return result;
 }
 
+/* See comment at builtin_test_op_file for differences between how
+ * GRUB implements file tests versus Petitboot's GRUB parser. */
 static bool builtin_test_op_dir(struct grub2_script *script, char op,
 		const char *dir)
 {
+	int rc;
+	struct stat statbuf;
+
 	if (op != 'd')
 		return false;
 
-	return parser_check_dir(script->ctx, script->ctx->device, dir) == 0;
+	rc = parser_stat_path(script->ctx, script->ctx->device, dir, &statbuf);
+	if (rc) {
+		return false;
+	}
+
+	return S_ISDIR(statbuf.st_mode);
 }
 
 static bool builtin_test_op(struct grub2_script *script,
diff --git a/discover/parser.c b/discover/parser.c
index fbf31b2..5598f96 100644
--- a/discover/parser.c
+++ b/discover/parser.c
@@ -1,8 +1,6 @@
 
 #include <fcntl.h>
 #include <stdlib.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 
 #include "types/types.h"
 #include <file/file.h>
@@ -49,24 +47,30 @@ int parser_request_file(struct discover_context *ctx,
 	return rc;
 }
 
-int parser_check_dir(struct discover_context *ctx,
-		struct discover_device *dev, const char *dirname)
+int parser_stat_path(struct discover_context *ctx,
+		struct discover_device *dev, const char *path,
+		struct stat *statbuf)
 {
-	struct stat statbuf;
-	char *path;
-	int rc;
+	int rc = -1;
+	char *full_path;
 
+	/* we only support local files at present */
 	if (!dev->mount_path)
 		return -1;
 
-	path = local_path(ctx, dev, dirname);
+	full_path = local_path(ctx, dev, path);
 
-	rc = stat(path, &statbuf);
-	talloc_free(path);
-	if (!rc)
-		return -1;
+	rc = stat(full_path, statbuf);
+	if (rc) {
+		rc = -1;
+		goto out;
+	}
 
-	return S_ISDIR(statbuf.st_mode) ? 0 : -1;
+	rc = 0;
+out:
+	talloc_free(full_path);
+
+	return rc;
 }
 
 int parser_replace_file(struct discover_context *ctx,
diff --git a/discover/parser.h b/discover/parser.h
index e0e8dc6..fc165c5 100644
--- a/discover/parser.h
+++ b/discover/parser.h
@@ -2,6 +2,9 @@
 #define _PARSER_H
 
 #include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
 
 #include "device-handler.h"
 
@@ -51,8 +54,9 @@ int parse_user_event(struct discover_context *ctx, struct event *event);
 /* File IO functions for parsers; these should be the only interface that
  * parsers use to access a device's filesystem.
  *
- * These are intended for small amounts of data, typically text configuration
- * and state files.
+ * These are intended for small amounts of data, typically text
+ * configuration and state files.  Note that parser_request_file,
+ * and parser_replace_file work only on non-directories.
  */
 int parser_request_file(struct discover_context *ctx,
 		struct discover_device *dev, const char *filename,
@@ -62,7 +66,15 @@ int parser_replace_file(struct discover_context *ctx,
 		char *buf, int len);
 int parser_request_url(struct discover_context *ctx, struct pb_url *url,
 		char **buf, int *len);
-int parser_check_dir(struct discover_context *ctx,
-		struct discover_device *dev, const char *dirname);
+/* parser_stat_path returns 0 if path can be stated on dev by the
+ * running user.  Note that this function follows symlinks, like the
+ * stat system call.  When the function returns 0, also fills in
+ * statbuf for the path.  Returns non-zero on error.  This function
+ * does not have the limitations on file size that the functions above
+ * do.  Unlike some of the functions above, this function also works
+ * on directories. */
+int parser_stat_path(struct discover_context *ctx,
+		struct discover_device *dev, const char *path,
+		struct stat *statbuf);
 
 #endif /* _PARSER_H */