| Message ID | 1356551618-8280-8-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | Tom Rini |
| Headers | show |
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.
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
-----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-----
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
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
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(-)