-----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