We messed up smbc_readdirplus :-(

Puran Chand puran157 at gmail.com
Tue Oct 15 07:50:28 UTC 2019


Hi Jeremy,

Added tests for smbc_readdirplus2(), also capturing timestamps, st_dev and
st_ino (if its not 0 in setup_stat() fn).
Please review and push if happy.

GitLab MR <https://gitlab.com/samba-team/samba/merge_requests/302/commits>.

-Puran

On Fri, Mar 15, 2019 at 2:28 PM Puran Chand <puran157 at gmail.com> wrote:

> Oops, forgot to attach patches.
>
> On Fri, Mar 15, 2019 at 2:27 PM Puran Chand <puran157 at gmail.com> wrote:
>
>> Hi Jeremy,
>>
>> I have added 2 patches here.
>> 1. moves generate_inode() and setup_stat() to util.c so that it can be
>> used by new API smbc_readdirplus2()
>> 2. New API  const struct libsmb_file_info *smbc_readdirplus2(unsigned int
>> dh, struct stat *st);   which fills up struct stat (from caller) and
>> returns libsmb_file_info (can be ignored by caller)
>>
>> Git Lab Merge request :-
>> https://gitlab.com/samba-team/samba/merge_requests/302
>>
>> Please review.
>>
>> -Puran
>>
>> On Tue, Feb 26, 2019 at 2:16 PM Puran Chand <puran157 at gmail.com> wrote:
>>
>>> Gentle reminder.
>>> Please review.
>>>
>>> On Mon, Jan 21, 2019 at 3:56 PM Puran Chand <puran157 at gmail.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Here is another patch over previous one.
>>>> This one moves generate_inode() and setup_stat() functions from
>>>> libsmb_stat.c to util.c so that the same can be used to create stat struct
>>>> from file_info struct for smbc_readdirplus2 (Work in Progress).
>>>> Also, removed SMBCCTX * from arguments for above both function as it
>>>> doesn't seem to serve any purpose.
>>>>
>>>> GitLab Link <https://gitlab.com/samba-team/samba/merge_requests/202/>
>>>>
>>>> -Puran
>>>>
>>>> On Thu, Jan 17, 2019 at 2:28 PM Puran Chand <puran157 at gmail.com> wrote:
>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> Created a patch to capture fileID as ino_t which was not captured
>>>>> earlier.
>>>>>
>>>>> I intend to move 'setup_stat()' and 'generate_inode()' from
>>>>> 'source3/libsmb/libsmb_stat.c' to util.c.
>>>>> This will help in converting 'struct libsmb_file_info' to 'struct
>>>>> stat' while retrieving 'struct stat' as part of new API smbc_readdirplus2()
>>>>> Also this will get us remaining missing information as well (st_dev,
>>>>> st_mode and st_blocks)
>>>>>
>>>>> GitLab Link <https://gitlab.com/samba-team/samba/merge_requests/199>
>>>>>
>>>>> -Puran
>>>>>
>>>>> On Wed, Jan 9, 2019 at 1:20 AM Jeremy Allison via samba-technical <
>>>>> samba-technical at lists.samba.org> wrote:
>>>>>
>>>>>> On Tue, Jan 08, 2019 at 06:38:52AM +0100, Andreas Schneider wrote:
>>>>>> > On Monday, 7 January 2019 23:20:30 CET Jeremy Allison wrote:
>>>>>> > > Hi Puran,
>>>>>> > >
>>>>>> > > I'm looking at Red Hat bug:
>>>>>> > >
>>>>>> > > https://bugzilla.redhat.com/show_bug.cgi?id=1569868
>>>>>> > >
>>>>>> > > and realized we made a mistake in the
>>>>>> > > implementation of smbc_readdirplus().
>>>>>> > >
>>>>>> > > struct libsmb_file_info
>>>>>> > >
>>>>>> > > should have been defined as a *superset*
>>>>>> > > of the stat struct that smbc_stat
>>>>>> > > returns, but currently it is missing
>>>>>> > > the st_ino, st_dev, st_mode and st_blocks
>>>>>> > > fields in order to be a superset of struct stat.
>>>>>> > >
>>>>>> > > We actually *have* this data as returned
>>>>>> > > from the SMB2_FIND_ID_BOTH_DIRECTORY_INFO
>>>>>> > > call when enumerating the directory, but
>>>>>> > > then throw it away when populating the
>>>>>> > > struct libsmb_file_info struct.
>>>>>> > >
>>>>>> > > I think the best way forward is to
>>>>>> > > add a smbc_readdirplus_ex() call that
>>>>>> > > returns a new 'struct libsmb_file_info_ex'
>>>>>> > > struct that includes these extra fields.
>>>>>> > >
>>>>>> > > Comments from other Samba Team members ?
>>>>>> >
>>>>>> > I would name it smbc_readdirplus2(), than you can implement 3 if we
>>>>>> still do
>>>>>> > something stupid (which I don't expect) but who knows :-)
>>>>>>
>>>>>> What I'll do is create a struct that contains
>>>>>> everything we can get back from SMB2_FIND_ID_BOTH_DIRECTORY_INFO,
>>>>>> and have it returned from smbc_readdirplus2().
>>>>>>
>>>>>> Internally I'll store this struct and then translate
>>>>>> to a returned struct libsmb_file_info for the
>>>>>> return from smbc_readdirplus().
>>>>>>
>>>>>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-Move-generate_inode-and-setup_stat-to-util.c.patch
Type: application/octet-stream
Size: 6255 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20191015/90939ced/0001-s3-Move-generate_inode-and-setup_stat-to-util.c.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-s3-Added-new-API-smbc_readdirplus2-which-fills-up-ca.patch
Type: application/octet-stream
Size: 23577 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20191015/90939ced/0002-s3-Added-new-API-smbc_readdirplus2-which-fills-up-ca.obj>


More information about the samba-technical mailing list