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

[Simon Glass]

Hi Tom,

On Fri, Mar 1, 2013 at 10:26 AM, Tom Rini <trini at 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 at 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 at 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 at 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