david_navigator

When I need to fix a bug in a method I try and think through all the places in the code that use that method, so that I can design my fix correctly (and then the tests won't fail), however as projects get bigger and methods inevitably call other methods, that gets harder and harder.
I tend to manually recursively use grep, e.g grep for calls to this method and then grep for calls to the methods that call this method etc etc, but that's time consuming and boring.

Does anyone know of any tool that can analyse a project and then show all the paths to a specific method ?

Ideally with some kind of family tree like output e.g so if I had to fix Display Value, I would have a reminder of all the places that relied on that method.

Ah thanks. I was looking for something to comment out the whole of a class in one go.

What does Text Tools | Comment Member(s) do ?

I can't see it in the documentation.

Thanks

David

24 minutes ago, David Heffernan said:

1. Why are you using WideString? That's a type for COM interop. Shouldn't temp be string?
2. You leak Buffer.
3. Why do you allocate buffer so that you can copy temp to Buffer and then to reply.buffer? Why not copy direct?
4. Why use Move and CopyMemory? They do the same job. Pick one and use it consistently.
5. What do you do to defend against buffer overrun of reply.buffer?
6. If your code raises an exception, that exception is raised at a specific line, and has a specific message. That information is useful. That you didn't include it in the post suggests that you have not studied it in detail. I bet that it tells you information that would help you understand. And certainly would help us understand. What is that information?

 

We can guess that temp[1] trips up range checking. But the exception message would remove any need for guessing. Btw, you fix that by using Pointer(temp)^ in place of temp[1].

Actually it's code buried deep in a third party component and I only looked at it because it started to error with D10.4.2
As I said, it worked (well at least functioned) correctly for the past 10 years under various versions of Delphi (up to 10.3)

>Why use Move and CopyMemory? They do the same job. Pick one and use it consistently.

I thought one dealt with overlapped memory & one didn't ? Though no idea why the original code used both.

I guess the 2 min fix to test that Test is not an empty string is going to turn in to a major rewrite. 

7 hours ago, emailx45 said:

as you should know, a "string" is as a "array"... then, if your (string = '') then, you array is count=0... where  temp[1] will be a index not valid!!!

then, it's good pratice verify this case:

if not(Temp='') then
begin
 ... do it
end;

 

hug

Absolutely. I can see why the code fails and how to fix it, what I don't understand is why it's been working for 10 years. This routine is called 100's of times a day and Temp is virtually always an empty string, but the exception only started when I compiled the app with D 10.4.2, so I suppose my question is, why ?
Maybe there's a compiler setting which has a different default in 10.4.2 ?

 move(temp[1],Buffer^, length(temp) * sizeof(WideChar));

procedure TnxdsrPluginCmdHandler.nmLoginMessage(var aMsg: TnxDataMessage);
var
  error: TnxResult;
  Buffer: PWideChar;
  BufLen: SmallInt;
  temp: WideString;
  reply: TnxnmServerlogonRpy;
begin
    BufLen := high(Reply.Buffer) + 1;
     Buffer :=AllocMem(BufLen);
    try
      if assigned(PluginEngine) then
        temp := WideString(TnxdsrServerPlugin(PluginEngine).LoginMessage)
      else
        temp := '';      
      move(temp[1],Buffer^, length(temp) * sizeof(WideChar));
      error := 0;
      BufLen := Length(Buffer);
      reply.buflen := buflen;
      copymemory(@reply.buffer, buffer, Buflen * sizeof(WideChar));
      assert(buflen = length(buffer), 'Data Checking data Lengths differ');
    except
      reply.BufLen := 0;
      error := DBIERR_NOSERVERSW;
    end;

Cheers

David

1 hour ago, Remy Lebeau said:

What's wrong with enumerating that folder? That is likely what the dialog is doing.  Why do you expect there to be an API that provides that list?  Not everything in a system UI has a dedicated API behind it.

Nothing. I just didn't want to do that IF there was an API that was the prefered method, especially as I couldn't find any documentation as to whether the folder name is localized in none English Windows.

Is there a better way to get the list of Windows sounds as displayed in this Sounds dropdown without simply enumerating the contents of the %windir%\Media folder ?

I think it's the sharing effect. I can't think of the number of times I've described an issue in a NG or a forum and realised the solution before I pressed submit.

Thanks for all the replies and suggestions. Just went for a walk with the dog and realised that I don't need the address info at all, I can index the other data fields (5 x integers) to find the record I need 🙂

Amazing how doing something different often brings alternative solutions to the forefront. 

1 hour ago, A.M. Hoornweg said:

Can you use the postal code to identify the town and street? 

 

Sadly not. In the USA Zip codes are not as precise as Netherlands or UK, as they refer to the sorting & delivery district, not the street.

3 minutes ago, Lars Fosdal said:

Even though they may refer to the same physical address?

 

Yes. The system that sends the address data is pretty good at making sure that there isn't more than one version of an address for a physical location and for those that did slip though it would be acceptable to have "duplicate" data in this table.

3 minutes ago, Lars Fosdal said:

Consider '1688 High St.' vs '1688 High Street' as an example.

In this particular scenario '1688 High St.' & '1688 High Street' would represent two distinctly different locations and thus two different records in the database. Even '1688 High St' (no period) would represent a third location.

I think I'm using the term HASH to refer to a checksum and was kind of hoping that different addresses would give different checksums. 
Kind of how What3Words works. 

Maybe I'm using the wrong terminology - this isn't a area I'm too clued up on.

I was hoping that irrespective of the length of the address I could calculate some value (of known max length) that uniquely represented the address and could be used in in the database table 

e.g in pseudo code

UNIQUEVALUE1  := Function ConvertAddress('1688highstknowlesolihullb930ly');

UNIQUEVALUE2  := Function ConvertAddress('1600pennsylvaniaavenuenwwashingtondc20500unitedstates');

DB TABLE

UNIQUEVALUE1 | UNIQUEVALUE2 | Float 1 | Float 2 | etc

22 minutes ago, David Heffernan said:

First of all, don't worry about hashing the data. Work out how to represent the data, what data structure to use. And then define what you mean by equality for this data. Once you have done that, and have a clear specification of that, defining a hash function should be obvious. But if not, we will be able to help at that point. But first you need to define the data structure and the equality operator.

Not sure I follow you.

I have two addresses and against those I need to store about 10 floats. This will all be in a database table.

The two addresses come from a third party app, they're sent as structured string data e.g
Field1 - address_line_1

Field2 - address_line_2

Field3 - Town

Fied4 - County

Field5 - PostCode

Field6 - Country

Fields can be empty.
There's no unique ID or anything useful, just a load of strings. The one thing that is consistent is that the same street address will always have the same field values.

Knowing the original source of the data, I think it's acceptable to concatenate all the fields into one string and remove all whitespace/punctuation.

Thus I could be sent

Field1 - 1688 High St.

Field3 - Knowle

Fied4 - Solihull

Field5 - B93 0LY

and convert it to 1688highstknowlesolihullb930ly

which I could then store as one of the keys e.g

Source | Destination | Float1 | Float 2 | etc

1688highstknowlesolihullb930ly | 1600pennsylvaniaavenuenwwashingtondc20500unitedstates | Float 1 | Float 2 | etc

But as there's no maximum length of any of the field data I'm sent, these keys could end up being unnecessarily long, so this doesn't feel like the right approach.

I need to store some data indexed on two physical street addresses - a source and a destination. Sometimes this street address might be as simple as just a Country, other times it could be a full Address down to the building's number.
I don't need to actually store the addresses.

In reality the source address will probably only have about a dozen different values, whereas the destination address will run in to 1000's.

I'm guessing that some kind of HASH is required of each address but this is an area that I don't really know much about and after any advice.

13 minutes ago, Lars Fosdal said:

Aside from that aPort should be a Word to avoid invalid port values above 65535 ... 

 

TMS FixInsight supports a hint for this, though: O804 Method parameter ''Foo'' is declared but never used

Thanks.... and had I declared it as a Word, then the compiler would have picked up that I'd accidentally added an extra zero in to the default value (160000, rather than 16000) and I wouldn't have needed to debug and thus create my own bug !!!

I just discovered a nasty bug in my code. During a previous debugging session I'd commented out the use of a parameter and replaced it with a hardcoded value i.e aPort replaced with 

16000
 

procedure TAPI_Handler.ConnectDatabase(const aIPaddress: string = '192.168.42.212'; const aPort: Integer = 160000);
...
...
...
FnxWinsockTransport.Port := 16000; // aPort;

Surprisingly the compiler doesn't issue a hint or a warning that aPort isn't used, neither does Pascal Analyzer. Does anyone know of any third party code checking tool (maybe TMS ?) that would have picked this up ?

Cheers

David

Quote

Although, i don't not quite understand this 8, is it 8 different types or used 8 times, with different named vars declared sets with the same type like set of integers...

 

8 different types.

Quote

What type of set ?

How many many different of them ?

Quickly Grepping the source, it looks like there are 8 independent sets that use the GetSetCardinality function.

Quote

This should do the trick:

Brilliant. Thank you very much. Saved me hours of googling & head scratching 🙂

Happy New Year.

Quote

Using a PByte would probably be better here 

 I think I might need some help here then.

The colleague that wrote this function is on furlough for at least the next four months due to Covid, so I have to address the problematic code myself and I'm not sure I understand why it's done like this to be able to refactor it correctly.

The function is passed a set along with the sizeof(the set)

This is the original function (with the array of bytes set at the original wrong value of 8 bytes, rather than I believe the correct value of 32 bytes)

function GetSetCardinality(const aSet; aLen: Word): Word;
{ Sample call would be:

  count := GetSetCardinality(SomeSet, sizeof(SomeSet)); }

var
  B, i: Integer;
  abyte: Byte;
  bytes: array [0 .. 7] of Byte absolute aSet; 
begin
  result := 0;
  for B := 0 to aLen - 1 do
  begin
    abyte := bytes[B];
    for i := 0 to 7 do
      if (abyte and (1 shl i)) <> 0 then
        Inc(result);
  end;
end;

Thanks for the replies.

The programmer's comment above the code indicates that the function is supposed to return the number of elements in a set. If I'm not mistaken I believe delphi allows up to 256 elements which are represented as 256 bits (rather than encoded in to 8 bytes), which I assume means that 

bytes: array [0 .. 7] of Byte absolute aSet;

should actually be

bytes: array [0 .. 31] of Byte absolute aSet;

The rest of the code simply counts the number of bits set in each abyte which it returns as the result.

I just compiled a colleagues code with Range checking turned on and got an immediate error.

It's fairly obvious why and an easy fix (aLen had a value of 9 in this circumstance and the bytes array is simply the wrong size), but I was wondering what actually happens when the faulty code is executing with range checking turned OFF ?

What actually happens when the line 

abyte := bytes[B]; 

is executed with a value of 8 for B ?
Is there a potential for memory corruption or does the code internally fail gracefully ?

function GetSetCardinality(const aSet; aLen: Word): Word;
var
  B, i: Integer;
  abyte: Byte;
  bytes: array [0 .. 7] of Byte absolute aSet; 
begin
  result := 0;
  for B := 0 to aLen - 1 do
  begin
    abyte := bytes[B];

...

...

Many thanks

Cheers

David