Do you consider this type construct to be a smelly code?

[Clément 148]

Hi,

 

I always tried to use the right type for the job, but there's a certain type construction that when used properly is just great.
Since this type construct uses pointers, I'm afraid it might die as pointers are evil (by some, I just can't imagine coding without them) and might get removed at some point.
Is there a better way to handle this construct?

 

function DoSomething( const Data; const DataSize : Cardinal ) : TBytes;
type
    PUInt64Array = ^TUInt64Array;
    TUInt64Array =  array[0..MaxInt div 16] of UInt64;

var

 {...};
begin
{.. some code ..}
	n := HashBlockSize div SizeOf(UInt64);
	for i := 0 to n-1 do begin
      PUInt64Array(@oString[0])[i] := PUInt64Array(@oString[0])[i] xor UInt64($5c5c5c5c5c5c5c5c);
      PUInt64Array(@iString[0])[i] := PUInt64Array(@iString[0])[i] xor UInt64($3636363636363636);
   end;
   {.. some more code .. }
end;

 

I'm using such construct as a helper for a HMAC_SHA256 implementation, and it works great. Do you consider it a smelly code? Is there a better way to reproduce without pointers?

 

TIA,

Clément
 

Edited June 27, 2019 by Clément

[Ugochukwu Mmaduekwe 42]

@Clément, To be sincere, I tend to keep pointer constructs to a minimum (to be on the safe side) since the size of integers varies on a various platforms supported by Delphi.

Also any reason why you reimplemented HMAC when it is supported in modern Delphi versions?

Edited June 27, 2019 by Ugochukwu Mmaduekwe

[Clément 148]

7 minutes ago, Ugochukwu Mmaduekwe said:

@Clément, To be sincere, I tend to keep pointer constructs to a minimum (to be on the safe side) since the size of integers varies on a various platforms supported by Delphi.

Also any reason why you reimplemented HMAC when it is supported in modern Delphi versions?

For this project I must use Delphi XE. Unfortunately most free implementation are not fast enough and/or depends on DLLs.
 

[Ugochukwu Mmaduekwe 42]

18 minutes ago, Clément said:

For this project I must use Delphi XE. Unfortunately most free implementation are not fast enough and/or depends on DLLs.
 

Well, have you tried Mormot's implementation?

 

https://github.com/synopse/SynPDF/blob/master/SynCrypto.pas

 

You can also try HashLib4Pascal if you want.

 

https://github.com/Xor-el/HashLib4Pascal/

 

Also you can try out Wolfgang Ehrhardt aka Gammatester (RIP) excellent Hash libraries which contains HMAC implementations too.

 

http://www.wolfgang-ehrhardt.de/crchash_en.html

 

These are guaranteed to work on Delphi 2010 and above and don't require any external DLLs for as long as I remember.

Edited June 27, 2019 by Ugochukwu Mmaduekwe

[Clément 148]

20 minutes ago, Ugochukwu Mmaduekwe said:

Well, have you tried Mormot's implementation?

 

https://github.com/synopse/SynPDF/blob/master/SynCrypto.pas

 

You can also try HashLib4Pascal if you want.

 

https://github.com/Xor-el/HashLib4Pascal/

 

Also you can try out Wolfgang Ehrhardt aka Gammatester (RIP) excellent Hash libraries which contains HMAC implementations too.

 

http://www.wolfgang-ehrhardt.de/crchash_en.html

 

These are guaranteed to work on Delphi 2010 and above and don't require any external DLLs for as long as I remember.

Thanks I'll have a look.

[Guest]

To answer the OQ (Original Question). The answer is "No" if you add comments to your code and name your methods "DoThis".

[Rollo62 538]

This looks like beautiful pointer and math magic to me.
If this works as expected, it would be worth to figure out why, you're right.
I never looked into crypto algorithms too deeply, but probably this could be a common practice to safe some computing power and/or memory

by fractioning or grouping the memory access somehow.

Maybe you can find similar structure in C++ or somewhere else too, with a little more description.

[Sherlock 663]

I only smell "magic numbers" there. Rest looks fine.

[David Heffernan 2347]

{$POINTERMATH ON} would be preferable, so avoiding the static array type 

[Hallvard Vassbotn 3]

Incomplete code?

 

Using pointers is fine(*), repeating code, especially inside a loop, is not. 

 

Move expressions like:

 

PUInt64Array(@oString[0])

 

outside the loop, assign to local vars. Then you can probably just use inc on the pointer vars instead of using slower array notation. 

 

(*) as long as you understand them, use them correctly, efficiently and readably. 

Edited June 28, 2019 by Hallvard Vassbotn

[Darian Miller 365]

Your question was "Is this type of construct smelly code?"  If you ever feel compelled to ask that about a specific chunk of code, then I would the assert that the obvious answer is YES.

[David Schwartz 427]

I suspect the main reason this occurs as "smelly" is because there are so few reasons to do this kind of thing in Delphi. It's far more common in c/c++ as old-timers think it's perfectly acceptable.

 

if you have a justifiable need to do bit-whacking like this, then I say, do it. I probably wouldn't do it for, say, a mere 5% performance gain, but 50%+ is far more justifiable.

 

It would be nice to see some comments as to what's supposed to be going on because average Delphi coders won't make heads or tails of the code. (I personally have trouble dealing with pointers in Delphi, since I do it so seldom.)

 

And be especially careful to point out any dependencies on assumed sizes of vars, especially integers that can vary depending on the platform.

 

(Better yet, use explicit types that ensure fixed sizes and explain why.)

Edited July 17, 2019 by David Schwartz