[PATCH] libsmbclient read-dir-plus

[Puran Chand]

Hi Jeremy,

I am sorry this delaying this for very long.

I raised another PR with changes for this at github
<https://github.com/samba-team/samba/pull/140>.

About missing/ignoring mode/uid/gid information from struct file_info,
those are already set and the same is stored in libsmb_file_info in
libsmb_dir.c::add_fileinfo. Please let me know if I am missing something
here.

I tried the other approach where smbc_readdirplus should return another
struct with stat and other fields as part of it (birth-time for now).
This would require me to create struct stat out of file_info in
libsmb_dir.c::add_fileinfo and add the same to libsmb_file_info struct.
I found reference code in libsmb_stat.c but I believe duplicating the same
code in libsmb_dir.c wouldn't be a good thing.
If I try to use the information from file_info only to create struct stat,
this would miss out few information in struct stat (ex:- st_ino, st_nlink)
which aren't part of file_info.

About birth_time macros, libsmbclient code will be executing on client side
and its fetching the information from SMB Server, hence the macros wouldn't
be of any help since we need to check whether the server supports
birth-time or not.
So regardless of whether SMB-Server supports birth-time, I am storing the
birth-time (which will be 0 if not supported by SMB Server). The same has
been documented in libsmbclient.h file as well.

I also am working on adding tests in libsmbclient.c and will share the same
asap.

-Puran


On Wed, Aug 30, 2017 at 10:32 AM, Puran Chand <puran157 at gmail.com> wrote:

> Thanks Jeremy for the review.
>
> I will do the required changes and get back to you.
>
> - Puran
>
> On Wed, Aug 30, 2017 at 1:06 AM, Jeremy Allison <jra at samba.org> wrote:
>
>> On Tue, Aug 29, 2017 at 11:36:48AM -0700, Jeremy Allison via
>> samba-technical wrote:
>> > On Wed, Aug 16, 2017 at 05:37:06PM +0530, Puran Chand wrote:
>> > > added patch file
>> >
>> > OK, I'm looking at this and it needs some changes. I'll get
>> > back to you soon with a modified version.
>>
>> OK, here is a modified version I want you to base
>> future changes on.
>>
>> First couple of things:
>>
>> 1). Removed (C) VMware statements you added on
>> every file touched. The git repo already will
>> track the VMware (C) and you've sent in the
>> form, so we don't need to litter the .c and .h
>> files with extra (C) statements.
>>
>> 2). Started using the modern Samba coding
>> style (DBG_XX statements instead of DEBUG,
>> modern spacing, one line per statement, etc.
>> etc.). libsmbclient is old, and has it's
>> own 'unique' style but we'll never fix it
>> to modern coding standards until we bite the
>> bullet and commit modern changes.
>>
>> 3). Re-using the client.h 'struct file_info'
>> as the struct to return from readdirplus
>> is the wrong decision. It's easy and convenient,
>> but ties the external API to the internal structs,
>> and they can get out of sync (expecially as you're
>> just re-using the definition). I changed this
>> to a struct libsmb_file_info that is only defined
>> inside libsmbclient and can be decoupled from
>> the internal definition.
>>
>> 4). Proving that file_info is the wrong
>> struct to use, you're returning DOS attribute
>> info in the 'mode' field of the struct,
>> as well as ignoring uid/gid. Rightly or
>> wrongly these are set to something reasonable
>> for the caller when you call libsmb_stat,
>> so these should be the same.
>>
>> 5). The best solution is to cause readdirplus
>> to return a structure containing the existing
>> system stat struct. smbc_stat() returns the
>> existing system stat struct, but wrapping it
>> in a containing struct for readdirplus allows
>> extra fields to be added/specified.
>>
>> If you need to return the create time, look into
>> the code inside source3/lib/system.c - make_create_timespec().
>> This shows all the #definitions needed to
>> determine if the system has birthtime, or
>> what varient it may have.
>>
>> 6). Add an unsigned int 'flags' field
>> to readdirplus, with only one current
>> bit - LIBSMB_READDIRPLUS_STAT (1), which
>> means return the stat struct. That way,
>> if desired we can add the ability to return
>> extra fields such as security descriptor,
>> extended attributes - etc. All the possible
>> fields you can get from the SMB2Search calls.
>>
>> Cheers,
>>
>> Jeremy.
>>
>
>