Patchwork [U-Boot,07/11] sandbox: Add a way of obtaining directory listings

login
register
mail settings
Submitter Simon Glass
Date Dec. 26, 2012, 7:53 p.m.
Message ID <1356551618-8280-8-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/208206/
State Accepted, archived
Delegated to: Tom Rini
Headers show

Comments

Simon Glass - Dec. 26, 2012, 7:53 p.m.
This implementation uses opendir()/readdir() to access the directory
information and then puts it in a linked list for the caller's use.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/sandbox/cpu/os.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/os.h          |   48 +++++++++++++++++++++++
 2 files changed, 149 insertions(+), 0 deletions(-)
Tom Rini - March 1, 2013, 5:37 p.m.
On Wed, Dec 26, 2012 at 11:53:34AM -0800, Simon Glass wrote:
> This implementation uses opendir()/readdir() to access the directory
> information and then puts it in a linked list for the caller's use.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/sandbox/cpu/os.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/os.h          |   48 +++++++++++++++++++++++
>  2 files changed, 149 insertions(+), 0 deletions(-)

Since the code looks fine,

Reviewed-by: Tom Rini <trini@ti.com>

But a question.  Do you really want this in cpu/os.c rather than some
new file for filesystem stuff (since this is the arch side of sandboxfs)
?  I can see you saying it should stay here since it's all OS
interaction related stuff.
Simon Glass - March 1, 2013, 6:21 p.m.
Hi Tom,

On Fri, Mar 1, 2013 at 9:37 AM, Tom Rini <trini@ti.com> wrote:
> On Wed, Dec 26, 2012 at 11:53:34AM -0800, Simon Glass wrote:
>> This implementation uses opendir()/readdir() to access the directory
>> information and then puts it in a linked list for the caller's use.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  arch/sandbox/cpu/os.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/os.h          |   48 +++++++++++++++++++++++
>>  2 files changed, 149 insertions(+), 0 deletions(-)
>
> Since the code looks fine,
>
> Reviewed-by: Tom Rini <trini@ti.com>
>
> But a question.  Do you really want this in cpu/os.c rather than some
> new file for filesystem stuff (since this is the arch side of sandboxfs)
> ?  I can see you saying it should stay here since it's all OS
> interaction related stuff.

Thanks for reviewing. The practical reason why everything is in os.c
is that this file is the interface between files which include
common.h and files which include system headers. But logically
speaking, I have tended to make os.c hold anything that interfaces
with or calls a Linux API function.

We could certainly create something like os_filedir,c or similar if
os.c is getting a bit large. But it would still need to include system
headers. I don't think we want anything like this in in drivers/ at
present.

Regards,
Simon

>
> --
> Tom
Tom Rini - March 1, 2013, 6:26 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/01/2013 01:21 PM, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, Mar 1, 2013 at 9:37 AM, Tom Rini <trini@ti.com> wrote:
>> On Wed, Dec 26, 2012 at 11:53:34AM -0800, Simon Glass wrote:
>>> This implementation uses opendir()/readdir() to access the
>>> directory information and then puts it in a linked list for the
>>> caller's use.
>>> 
>>> Signed-off-by: Simon Glass <sjg@chromium.org> --- 
>>> arch/sandbox/cpu/os.c |  101
>>> +++++++++++++++++++++++++++++++++++++++++++++++++ include/os.h
>>> |   48 +++++++++++++++++++++++ 2 files changed, 149
>>> insertions(+), 0 deletions(-)
>> 
>> Since the code looks fine,
>> 
>> Reviewed-by: Tom Rini <trini@ti.com>
>> 
>> But a question.  Do you really want this in cpu/os.c rather than
>> some new file for filesystem stuff (since this is the arch side
>> of sandboxfs) ?  I can see you saying it should stay here since
>> it's all OS interaction related stuff.
> 
> Thanks for reviewing. The practical reason why everything is in
> os.c is that this file is the interface between files which
> include common.h and files which include system headers. But
> logically speaking, I have tended to make os.c hold anything that
> interfaces with or calls a Linux API function.
> 
> We could certainly create something like os_filedir,c or similar
> if os.c is getting a bit large. But it would still need to include
> system headers. I don't think we want anything like this in in
> drivers/ at present.

I agree with not putting this into drivers/ as it's sandbox-side
stuff.  If os.c isn't yet unwieldy to you, OK, we can go as-is.  But
I'll ask next time you add another big hunk to os.c I'll ask if it's
unwieldy yet.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRMPLmAAoJENk4IS6UOR1W+1YP/Rveu5b/OoEPqTqo6akhTiZa
chVnl0td2tFYW6SQmLfCn8qPLBjsprsxlUzAywkvBzn8HJ33wPtKx00vCj04m9Fw
ELbFuGpEkLEkHL6UEF2j9PGtZKudqybl7m0RRWIZfVA3ku4rPvav0w59WFzhNKF7
5EqzMyfvVX9XlnkVJ6H75H/JkNFxF3DaJ9jYs7zeOxmlyhgPby4df8GQEENpOvRs
co2DvlbcuT3pHXAW7jqUYMb3wGphvCKlb2NE8fOjPszCm9F5En/M3i01lKFVRPZ1
MSLszZaUmMm6K4UjLVdnAAl8lPmP7SdwpsH0EhmehsVlh7jshbjhbEgAcN4tr2+V
nUP/yW8gJl3oJ0JL04HV0ggYY7U71WnTRC+AMJcFdfen/4btfqGkJbERp5fiO6La
qO8kx+mhl4hycMB+2pAhux6b4wK+vPXuyK7QbnliKA715S9FK3huf/u/QiPntKMA
6kE23ROuZMzvpnRu39brPCSAXLjDgyxWR+ofT/BuKPyrcXvOFjTa+IyI3EcFPKFX
0CdaGU6b/cxBVOHkBBeRLq0a5I2NNjV3dEXW78B9HmRNSpeDpCiZrNKDOe5ov1XH
MVqthVBxQWgvoSZrQ+t4ff8dnP2fjNJfaUn15+EzmM8YZDPownNlaLaiSMuKL3dR
XmF60m4s5kHSYpLOWa9l
=jsTJ
-----END PGP SIGNATURE-----
Simon Glass - March 1, 2013, 6:27 p.m.
Hi Tom,

On Fri, Mar 1, 2013 at 10:26 AM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 03/01/2013 01:21 PM, Simon Glass wrote:
>> Hi Tom,
>>
>> On Fri, Mar 1, 2013 at 9:37 AM, Tom Rini <trini@ti.com> wrote:
>>> On Wed, Dec 26, 2012 at 11:53:34AM -0800, Simon Glass wrote:
>>>> This implementation uses opendir()/readdir() to access the
>>>> directory information and then puts it in a linked list for the
>>>> caller's use.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org> ---
>>>> arch/sandbox/cpu/os.c |  101
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++ include/os.h
>>>> |   48 +++++++++++++++++++++++ 2 files changed, 149
>>>> insertions(+), 0 deletions(-)
>>>
>>> Since the code looks fine,
>>>
>>> Reviewed-by: Tom Rini <trini@ti.com>
>>>
>>> But a question.  Do you really want this in cpu/os.c rather than
>>> some new file for filesystem stuff (since this is the arch side
>>> of sandboxfs) ?  I can see you saying it should stay here since
>>> it's all OS interaction related stuff.
>>
>> Thanks for reviewing. The practical reason why everything is in
>> os.c is that this file is the interface between files which
>> include common.h and files which include system headers. But
>> logically speaking, I have tended to make os.c hold anything that
>> interfaces with or calls a Linux API function.
>>
>> We could certainly create something like os_filedir,c or similar
>> if os.c is getting a bit large. But it would still need to include
>> system headers. I don't think we want anything like this in in
>> drivers/ at present.
>
> I agree with not putting this into drivers/ as it's sandbox-side
> stuff.  If os.c isn't yet unwieldy to you, OK, we can go as-is.  But
> I'll ask next time you add another big hunk to os.c I'll ask if it's
> unwieldy yet.

Understood, thanks.

Regards,
Simon

Patch

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 3e37c93..d075407 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -19,10 +19,13 @@ 
  * MA 02111-1307 USA
  */
 
+#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
+#include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <termios.h>
 #include <time.h>
 #include <unistd.h>
@@ -261,3 +264,101 @@  int os_parse_args(struct sandbox_state *state, int argc, char *argv[])
 
 	return 0;
 }
+
+void os_dirent_free(struct os_dirent_node *node)
+{
+	struct os_dirent_node *next;
+
+	while (node) {
+		next = node->next;
+		free(node);
+		node = next;
+	}
+}
+
+int os_dirent_ls(const char *dirname, struct os_dirent_node **headp)
+{
+	struct dirent entry, *result;
+	struct os_dirent_node *head, *node, *next;
+	struct stat buf;
+	DIR *dir;
+	int ret;
+	char *fname;
+	int len;
+
+	*headp = NULL;
+	dir = opendir(dirname);
+	if (!dir)
+		return -1;
+
+	/* Create a buffer for the maximum filename length */
+	len = sizeof(entry.d_name) + strlen(dirname) + 2;
+	fname = malloc(len);
+	if (!fname) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	for (node = head = NULL;; node = next) {
+		ret = readdir_r(dir, &entry, &result);
+		if (ret || !result)
+			break;
+		next = malloc(sizeof(*node) + strlen(entry.d_name) + 1);
+		if (!next) {
+			os_dirent_free(head);
+			ret = -ENOMEM;
+			goto done;
+		}
+		strcpy(next->name, entry.d_name);
+		switch (entry.d_type) {
+		case DT_REG:
+			next->type = OS_FILET_REG;
+			break;
+		case DT_DIR:
+			next->type = OS_FILET_DIR;
+			break;
+		case DT_LNK:
+			next->type = OS_FILET_LNK;
+			break;
+		}
+		next->size = 0;
+		snprintf(fname, len, "%s/%s", dirname, next->name);
+		if (!stat(fname, &buf))
+			next->size = buf.st_size;
+		if (node)
+			node->next = next;
+		if (!head)
+			head = node;
+	}
+	*headp = head;
+
+done:
+	closedir(dir);
+	return ret;
+}
+
+const char *os_dirent_typename[OS_FILET_COUNT] = {
+	"   ",
+	"SYM",
+	"DIR",
+	"???",
+};
+
+const char *os_dirent_get_typename(enum os_dirent_t type)
+{
+	if (type >= 0 && type < OS_FILET_COUNT)
+		return os_dirent_typename[type];
+
+	return os_dirent_typename[OS_FILET_UNKNOWN];
+}
+
+ssize_t os_get_filesize(const char *fname)
+{
+	struct stat buf;
+	int ret;
+
+	ret = stat(fname, &buf);
+	if (ret)
+		return ret;
+	return buf.st_size;
+}
diff --git a/include/os.h b/include/os.h
index c452d1b..038aba9 100644
--- a/include/os.h
+++ b/include/os.h
@@ -146,4 +146,52 @@  u64 os_get_nsec(void);
  */
 int os_parse_args(struct sandbox_state *state, int argc, char *argv[]);
 
+/*
+ * Types of directory entry that we support. See also os_dirent_typename in
+ * the C file.
+ */
+enum os_dirent_t {
+	OS_FILET_REG,		/* Regular file */
+	OS_FILET_LNK,		/* Symbolic link */
+	OS_FILET_DIR,		/* Directory */
+	OS_FILET_UNKNOWN,	/* Something else */
+
+	OS_FILET_COUNT,
+};
+
+/** A directory entry node, containing information about a single dirent */
+struct os_dirent_node {
+	struct os_dirent_node *next;	/* Pointer to next node, or NULL */
+	ulong size;			/* Size of file in bytes */
+	enum os_dirent_t type;		/* Type of entry */
+	char name[0];			/* Name of entry */
+};
+
+/**
+ * Get a directionry listing
+ *
+ * This allocates and returns a linked list containing the directory listing.
+ *
+ * @param dirname	Directory to examine
+ * @param headp		Returns pointer to head of linked list, or NULL if none
+ * @return 0 if ok, -ve on error
+ */
+int os_dirent_ls(const char *dirname, struct os_dirent_node **headp);
+
+/**
+ * Get the name of a directory entry type
+ *
+ * @param type		Type to cehck
+ * @return string containing the name of that type, or "???" if none/invalid
+ */
+const char *os_dirent_get_typename(enum os_dirent_t type);
+
+/**
+ * Get the size of a file
+ *
+ * @param fname		Filename to check
+ * @return size of file, or -1 if an error ocurred
+ */
+ssize_t os_get_filesize(const char *fname);
+
 #endif