Board index » cppbuilder » Is this safe ? property getter and setter for char*

2007-07-05 06:29:17 AM
cppbuilder27
Hi there
I have been away fro programming for a while so I am a little rusti..;-)
So I would like to know if the code below is safe to use.
There is of cource a lot more code, I just tok out an example.
Thanks in advance
Asger
h file
struct TC5L01
{
char Lengde[6];
char Text2[36];
};
class TC5Vare
{
private:
TC5L01* FC5L01;
char* __fastcall getLengde(){return FC5L01->Lengde;};
void __fastcall setLengde(char* Value);
char* __fastcall getText2(){return FC5L01->Text2;};
public:
__property char* Lengde = {read=getLengde, write=setLengde};
__property char* Text2 = {read=getText2};
};
cpp file:
//---------------------------------------------------------------------------
void __fastcall TC5Vare::setLengde(char* Value)
{
char* Str = Value;
char* L = FC5L01->Lengde;
if(Str[0] == '0')
{
L[0] = Str[1];
L[1] = '.';
L[2] = Str[2];
L[3] = Str[3];
}else{
L[0] = Str[0];
L[1] = Str[1];
L[2] = '.';
L[3] = Str[2];
L[4] = Str[3];
}
}
//---------------------------------------------------------------------------
Using the properties: (i am reading a long text file: char* Src)
Lengde = &Scr[16];
strcpy(Text1, &Scr[72]);
 

I guess in the expression
strcpy(Text1, &Scr[72]);
Text1 is typo? Did you mean the Text2? If so then IHMO this expression is
not safe. With this expression you can overwrite the memory that follows
after Text2.
Vladimir Grigoriev
"Asger Jorgensen" < XXXX@XXXXX.COM >wrote in message

Also I would append the character string pointed by L (FC5L01->Lengde) with
zeroes. I.e. before assignments I would set the whole memory (6 bytes) with
zeroes by using memset() function.
Vladimir Grigoriev
"Asger Jorgensen" < XXXX@XXXXX.COM >wrote in message

{smallsort}

Also I don't see a need to introduce additional pointers Str and L. Moreover
I would define L as follows
char * const L = FC5L01->Lengde;
Vladimir Grigoriev
"Asger Jorgensen" < XXXX@XXXXX.COM >wrote in message

"Vladimir Grigoriev" < XXXX@XXXXX.COM >wrote in message
it safer. Second choice would be AnsiString. Why use
char* here?
 

Hi Vladimir and Duane
And thanks to both of You!
You have answered my question, mainly my concerne was if
i could be sure that it was the right pointer i was working on
using __property and all.
To answer Your questions.
Yes Text1 is a typo both Text1 and 2 is in the code, I just
copied the wwrong one.
Yes every thing is memset, I memset the whole struct before
starting.
And the part about overwriting:
The real code looks like this:
Tmp = Scr[60];
Scr[60] = '\0';
char* S = TrimStr(&Scr[25], &Scr[60]);
if(S != '\0')
{
FixDanskeChars(S); // A/N 35
strcpy(Text1, S);
}
Scr[60] = Tmp;
The reason i use char* are speed and owerhead.
The program is going to be used as a fast reprecentation
of invoices and there might be 1 - 5000 invoices all
containing from 1 - 20 lines of data, so it is a lot
of data ( char*, double int )
And I find the char* easier to work with, thats what
we learned in the old days..;-)
But isn't it a little bit faster to use a direct pointer
then it is to use char* L instead of FC5L01->Lengde ??
This is done in every line in every invoice.
Thanks again
Kind regards
Asger
 

Asger Jorgensen < XXXX@XXXXX.COM >writes:
a constant problem for programmers in general. So in that regard, the
choice of char* is less safe than that of using a string class.
This code:
void __fastcall TC5Vare::setLengde(char* Value)
{
char* Str = Value;
char* L = FC5L01->Lengde;
if(Str[0] == '0')
{
L[0] = Str[1];
L[1] = '.';
L[2] = Str[2];
L[3] = Str[3];
}else{
L[0] = Str[0];
L[1] = Str[1];
L[2] = '.';
L[3] = Str[2];
L[4] = Str[3];
}
}
Is not really safe, since the user provides the address that Value
holds, and it could be null or invalid. It's easy to check for null,
but impossible to check for invalid. However, if used correctly, this
function would be safe.
However, for this code:
Lengde = &Scr[16];
strcpy(Text1, &Scr[72]);
I have stronger objections. Primarily, strcpy could overrun the
buffer. I'm not sure what these two lines have to do with the
previous code, except they both have something named Lengde in them.
Overall, I would suggest you use a string class. I don't see any
compelling reason to stick with raw pointers, and the downside is
big.
--
Chris (TeamB);
 

"Asger Jorgensen" < XXXX@XXXXX.COM >wrote in message
that with no problems. You may want to benchmark
this before assuming that char* is going to be faster.
If you only need windows and no portability, you should
at least consider AnsiString. Given Borland's optimization
with std stuff, std::string may be slower.
I think that if you add the code that you need to make
your char* implementation safe and THEN compare it
to an AnsiString implementation you will be surprised.
I would certainly advise testing this. It shouldn't be much to
write a similar module using AnsiString and friends.