[PATCH] libsmbclient read-dir-plus

Puran Chand puran157 at gmail.com
Wed Aug 30 05:02:24 UTC 2017


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


More information about the samba-technical mailing list