[PATCH] libsmbclient: readdirplus call.

[Jeremy Allison]

On Fri, May 04, 2018 at 09:36:22AM +0300, Alexander Bokovoy wrote:
> 
> I have few comments where I'd like to understand whether it is by
> design or a copy/paste mistake.

No problem !

> > >  	finfo->atime_ts = interpret_long_date((const char *)dir_data + 16);
> > >  	finfo->mtime_ts = interpret_long_date((const char *)dir_data + 24);
> > >  	finfo->ctime_ts = interpret_long_date((const char *)dir_data + 32);
> > > diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c
> > > index 41f585120b9..5cb1fce4338 100644
> > > --- a/source3/libsmb/clilist.c
> > > +++ b/source3/libsmb/clilist.c
> > > @@ -77,6 +77,14 @@ static size_t interpret_long_filename(TALLOC_CTX *ctx,
> > >  			if (pdata_end - base < 27) {
> > >  				return pdata_end - base;
> > >  			}
> > > +			/*
> > > +			 * What we're returning here as ctime_ts is
> > > +			 * actually the server create time.
> > > +			 */
> > > +			finfo->btime_ts = convert_time_t_to_timespec(
> > > +				make_unix_date2(p+4,
> > > +					smb1cli_conn_server_time_zone(
> > > +						cli->conn)));
> > >  			finfo->ctime_ts = convert_time_t_to_timespec(
> > >  				make_unix_date2(p+4, smb1cli_conn_server_time_zone(cli->conn)));
> What about here? Is it intended to provide the same value as ctime_ts
> instead of stating that we didn't get any birth time at all?
> SMB_FIND_INFO_STANDARD level doesn't give you that information, unlike
> SMB_FIND_FILE_BOTH_DIRECTORY_INFO.

But we *did* get the birthtime :-). That's what the comment is
trying to say.

From MS-CIFS:

2.2.8.1 FIND Information Levels
2.2.8.1.1 SMB_INFO_STANDARD
This information level structure is used in TRANS2_FIND_FIRST2 (section 2.2.6.2) and
TRANS2_FIND_NEXT2 (section 2.2.6.3) responses to return the following information for all files that
match the request's search criteria:
 Creation, access, and last write timestamps
 File size
 File attributes
 File name
SMB_INFO_STANDARD[SearchCount]
{
ULONG ResumeKey (optional);
SMB_DATE CreationDate;
SMB_TIME CreationTime;
SMB_DATE LastAccessDate;
SMB_TIME LastAccessTime;
SMB_DATE LastWriteDate;
SMB_TIME LastWriteTime;
ULONG FileDataSize;
ULONG AllocationSize;
SMB_FILE_ATTRIBUTES Attributes;
UCHAR FileNameLength;
SMB_STRING FileName;
}
..
CreationDate: (2 bytes): This field contains the date when the file was created.
CreationTime: (2 bytes): This field contains the time when the file was created.

What we are already returning as change time - ctime, is actually already the creation
time as that's all this call provides from the server. So it's OK to
actually return it in the correct field (IMHO).

This text came from Chris, who (hopefully) got it right by
reading the actual Microsoft source code.

I really hope so, as putting birth time there is what smbd does
as a server :-).

As a future experiment, we should probably test this against
a real Windows box and see if this date changes. If it does
then we have the mother of all Dochelp bugs :-).

> > >  			finfo->atime_ts = convert_time_t_to_timespec(
> > > @@ -128,6 +136,14 @@ static size_t interpret_long_filename(TALLOC_CTX *ctx,
> > >  			if (pdata_end - base < 31) {
> > >  				return pdata_end - base;
> > >  			}
> > > +			/*
> > > +			 * What we're returning here as ctime_ts is
> > > +			 * actually the server create time.
> > > +			 */
> > > +			finfo->btime_ts = convert_time_t_to_timespec(
> > > +				make_unix_date2(p+4,
> > > +					smb1cli_conn_server_time_zone(
> > > +						cli->conn)));
> > >  			finfo->ctime_ts = convert_time_t_to_timespec(
> > >  				make_unix_date2(p+4, smb1cli_conn_server_time_zone(cli->conn)));
> Same here.
> 
> > >  			finfo->atime_ts = convert_time_t_to_timespec(
> > > @@ -250,6 +266,9 @@ static bool interpret_short_filename(TALLOC_CTX *ctx,
> > >  
> > >  	finfo->mode = CVAL(p,21);
> > >  
> > > +	/* We don't get birth time. */
> > > +	finfo->btime_ts.tv_sec = 0;
> > > +	finfo->btime_ts.tv_nsec = 0;
> So here we default to 0 but in the cases above we don't default to zero.
> It is confusing and I'd like you to explain why we have different
> defaults.

So for the old DOS "search" call, which is what this code
copes with we don't get a birth time, so fill in as zero.

Hope this explaination helps. Let me know if you're OK
with the code now or you need more info.

Cheers,

	Jeremy.