Jump to content
Kyle_Katarn31

Weird code in THttpConnection.ProcessWellKnownDir

Recommended Posts

Some observations :

{ V8.65 look for absolute URL sent by proxy }
    I := Pos('://', FPath);
    if (I = 4) or (I = 5) then begin
        FPath := Copy(FPath, I + 3, 99999);  // strip http://
        I := Pos('/', FPath);  // start of path
        if (I > 1) then
            FPath := Copy(FPath, I, 999999);  // strip host
    end;

Magic numbers 99999 and 999999 to be replaced with actual expected FPath extract length ?

 

 else begin
                { see if we have access to file, but don't read it yet }
                TempStream := TFileStream.Create(FDocument, fmOpenRead + fmShareDenyWrite);
                TempStream.Destroy;
                OK := TRUE;
            end;

Destroy is called even if .Create fails and the global code section is a nonsense as always returns OK = TRUE whatever the access rights 

 

TempoStream.Free shall be called rather than TempoStream.Destroy

Edited by Kyle_Katarn31

Share this post


Link to post

It won't copy more characters than are in the string. Passing a large number for count is basically saying copy until the end is reached. 

 

However if the count parameter is left out the compiler will put in max int behind the scenes leading to cleaner code and no problematic magic value that could truncate a really long string.

 FPath := Copy(FPath, I + 3);

Share this post


Link to post
2 minutes ago, Brian Evans said:

It won't copy more characters than are in the string. Passing a large number for count is basically saying copy until the end is reached. 

 

However if the count parameter is left out the compiler will put in max int behind the scenes leading to cleaner code and no problematic magic value that could truncate a really long string.


 FPath := Copy(FPath, I + 3);

That's exactly my point.

Share this post


Link to post
11 hours ago, Kyle_Katarn31 said:

Destroy is called even if .Create fails

This statement is incorrect. The only way for Create to fail is if it raises an exception. The code is odd, but it doesn't do what you say. 

Share this post


Link to post
18 minutes ago, David Heffernan said:

This statement is incorrect. The only way for Create to fail is if it raises an exception. The code is odd, but it doesn't do what you say. 

Yes, right.

Anyway TempoStream.Free shall be called rather than TempoStream.Destroy

Share this post


Link to post
36 minutes ago, Kyle_Katarn31 said:

TempoStream.Free shall be called rather than TempoStream.Destroy

That's what I would do, but it's just a needless nil check which a compiler could remove. 

Share this post


Link to post
1 hour ago, Kyle_Katarn31 said:

Ok. So at least the Copy enhancement to be considered ?

I don't know what you mean here. Did you write this code? What problems are you having with the code? 

Share this post


Link to post
1 hour ago, Kyle_Katarn31 said:

Ok. So at least the Copy enhancement to be considered ?

I'm not sure at all that the old Delphi compilers ICS still support have an option 3rd argument.

 

Share this post


Link to post

It doesn't really matter what you use, as long as the value is "large enough".  If the value you specify is more than the number of characters actually available, Copy() will just stop when it reaches the end of the string. So, whether you use:

FPath := Copy(FPath, I + 3);

Or

FPath := Copy(FPath, I + 3, MaxInt);

Or

FPath := Copy(FPath, I + 3, Length(FPath));

Or

FPath := Copy(FPath, I + 3, Length(FPath) - I + 3);

It is all the same as far as Copy() cares.  Personally, I use MaxInt.

  • Like 1

Share this post


Link to post

I too use MaxInt for new applications, but I've no idea whether it is available in all the ancient compilers ICS still supports, so always keep code a simple as possible. 

 

There is no complete list of new features, functions and constants added with each new release,  When I needed to use PosEx to simplify some code, I had to use three conditionals as it changed three times since Delphi 7, had to search old source directories to find out when, which all takes valuable time.

 

This won't matter for much longer since the next major release of ICS will cease support for compilers more than a few years old, so we can finally make use of new language features and remove lots of ANSI specific stuff.  But we will changing too much old working code, since invariably changes introduce errors and often new things don't work properly.

 

For instance when using TSearchRec to index files, I tried using the new TimeStamp property that returns TDateTime instead of Time that returns a file date stamp, but eventually discovered TimeStamp returns UTC time instead of local time, both undocumented in help so don't know if this is deliberate or a bug. 

 

But it meant my applications could not correctly compare source and destination directories when FTP or copying files, causing hundred of thousands of extra files to be copied, until I reverted to using Time.  So new is not always better.

 

Angus

 

 

 

 

  • Like 2

Share this post


Link to post

Thanks

"This won't matter for much longer since the next major release of ICS will cease support for compilers more than a few years old"

 

You mean that you are about to drop Delphi 7 support ? What a bad news !

 

Share this post


Link to post
6 minutes ago, Kyle_Katarn31 said:

What a bad news !

The old code won't stop working. It's unrealistic to expect developers to expend extra time supporting legacy compilers and systems. It's no fun doing that. 

  • Like 3

Share this post


Link to post

No decision yet on how many old releases the next major version of ICS will support, partly down to age of old new language features, and partly because each time I add a new unit I have to update 100 to 200 .dpr, .dprog and .cprog files, something we should have automated a long time ago, but it's quicker to do it manually each time...

 

ICS V7 will still be supported for new compilers and OpenSSL versions (unless totally breaking) and major bugs, but no new components or features, unless another contributor takes over that support and testing, I've now migrated all my D2007 projects to D11 so don't need to use D2007 any longer. 

 

Angus

 

Share this post


Link to post

The new version I started to develop take 90% of existing ICS code. The new code is developed with Delphi 11 and no effort is done to support older compilers. Since I keep Delphi always up-to-date, it is likely that at the time of release, the next ICS version AKA V9 will officially and initially only support the latest Delphi at that time.

Share this post


Link to post

D7 has MaxInt for sure as I use it everywhere in Copy. I really can't understand why Borland didn't make 3rd parameter optional.

That magic constant likely has its origin from the very ancient versions. However, that could be High(Integer) instead...

On 10/3/2022 at 4:28 PM, Lars Fosdal said:

Legacy = Age > 4 years 😛

4 is magic constant defined personally by yourself 😛

Edited by Fr0sT.Brutal

Share this post


Link to post

That's an estimate based on when the average time before the warranty on a laptop runs out and updates are no longer provided 😛

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×