Valhalla Legends Forums Archive | C/C++ Programming | Weird, is there anything wrong with this function??

AuthorMessageTime
BreW
[code]char *StrToHex(char *input) {
char sdfg[4], outputbuf[2048];
for (unsigned int i = 0; i != strlen(input); i++) {
AddChat(vbRed, (char *)input[i]);
sprintf(sdfg, "%02X", input[i]);
AddChat(vbRed, sdfg);
sdfg[2] = 0x20;
sdfg[3] = 0x00;
strcpy(outputbuf + (i * 3), sdfg);
}
*(outputbuf + (i * 3) + 1) = 0x00;
void *asdf = malloc(sizeof(outputbuf));
strcpy((char *)asdf, (const char *)outputbuf);
return (char *)asdf;
}[/code]
well? i can't seem to see what's wrong
also i get
error C2440: '=' : cannot convert from 'char *' to 'char [256]'
here's how i'm calling it:
[code]
char asdf[256];
char sdfg[256];
strcpy(asdf, "yo dawg");
sdfg = StrToHex(asdf);
AddChat(vbOrange, sdfg);
[/code]
if i'm not mistaken, refering to an array without a specified index is the same as saying the base address of the array, which is also a char pointer. So uh..... why can't it convert it? Also for some reason it only started bugging me about it now, i've called plenty of char pointer functions just like that and it always worked for me :(
July 23, 2007, 8:37 PM
UserLoser
Oh wow, "vbRed, vbOrange".  You are NOT using VB so stop using VB naming conventions.  void *asdf doesn't look right.

Try strcpy(sdfg, StrToHex("yo dawg")); ?
July 23, 2007, 10:08 PM
BreW
[quote author=UserLoser link=topic=16901.msg171166#msg171166 date=1185228495]
Oh wow, "vbRed, vbOrange".  You are NOT using VB so stop using VB naming conventions. 
[/quote]
I can't really help that, i've been using vb for so long it's almost impossible for me to stop using those naming conventions... also among other things i defined are my own C++ Left(), Mid(), InStr(), CStr(), StrReverse(), so on.

[quote]
void *asdf doesn't look right.
[/quote]
It should, because that's the return type of malloc() in Visual C++ and if you check you'll see something like "void *malloc(size_t)" so i figured i should use void *. There's nothing wrong with it.... i use it in other functions that way and it works just fine

[quote]
Try strcpy(sdfg, StrToHex("yo dawg")); ?
[/quote]
Didn't help any. :/

By the way, I found the problem, "AddChat(vbRed, (char *)input[ i ]);" was what's causing it to crash, all i wanted it to do is to addchat the current letter. any idea how to properly do this? if i dont use the array element specifier, wouldn't it just print out everything from that index to the next null char?... so that's why i specified the index and made it a (char *) but i bet there's more to it, and i don't feel like trying to make something like that working right now. Well, it seems to be working except when i try to use it to output what's in my packet buffer (it just crashes.)

[code]
void SendPacket(char PacketID) {
char tmpbuf[2048];//, lenbuf[3];
tmpbuf[0] = (unsigned char)0xFF;
tmpbuf[1] = (unsigned char)PacketID;
strcpy(tmpbuf + 2, MakeWORD(length));
strcpy(tmpbuf + 4, buffer);
char *sdfg = StrToHex(tmpbuf); <------------ line that crashes
AddChat(vbYellow, sdfg);
//send(s, tmpbuf, length + 4, 0);
}
[/code]
where buffer is a global 2044 element char array. :-/
July 23, 2007, 10:42 PM
l2k-Shadow
well besides your current problem that function won't work.

[code]
        strcpy(tmpbuf + 2, MakeWORD(length));
strcpy(tmpbuf + 4, buffer);
[/code]

lol ::) take a look at how strcpy() works.

EDIT:
also, why are you doing this so complicated...
[code]
void StrToHex(const char* in, char* out)
{
memset(out, 0, 1);
char t[4];
for (int i = 0; i < strlen(in); i++)
{
sprintf(t, "%02X ", in[i]);
strcat(out, t);
}
}

void main()
{
char out[32];
StrToHex("TEST TEST!", out);
cout << out << endl;
}
[/code]
July 23, 2007, 11:20 PM
BreW
[quote author=l2k-Shadow link=topic=16901.msg171168#msg171168 date=1185232801]
well besides your current problem that function won't work.

[code]
        strcpy(tmpbuf + 2, MakeWORD(length));
strcpy(tmpbuf + 4, buffer);
[/code]

lol ::) take a look at how strcpy() works.
[/quote]
How does strcpy work and why would I need to know? I don't see anything wrong with that code--if you say there's something flawed with that, how would you write it?

[quote]
EDIT:
also, why are you doing this so complicated...
[code]
void StrToHex(const char* in, char* out)
{
memset(out, 0, 1);
char t[4];
for (int i = 0; i < strlen(in); i++)
{
sprintf(t, "%02X ", in[i]);
strcat(out, t);
}
}

void main()
{
char out[32];
StrToHex("TEST TEST!", out);
cout << out << endl;
}
[/code]
[/quote]
Okay, so maybe I did overlook adding the space to the string to be formatted, but your version uses strcat(); which I have been trying to avoid. It gives me some error about ESP and calling conventions (i forget the rest of it) when I use strcat in that function. Also in the for loop for the conditional, i've noticed you're using "i < strlen(in)", which really should be avoided unless completely necessary-- it takes less cpu cycles to check if a variable is not equal to another rather then to check if it's greater then a specified value, where my way would indeed be much more efficient, and let's face it. if we're going to make a program in C/C++, it's probably because it's so much more efficient of a language, so why not make the code efficient too? Maybe the compiler may optimize that piece of code, but in my opinion it's always good to be on the safe side. I don't really see how your version is much better then mine. And half the code that's "gone" in your version is because it modifies an output argument byref, and that's what I'm trying not to do. I want to specifically return a pointer to the output. Maybe your version looks cleaner, shorter variable names, and works, but I got my version working just fine too.

EDIT**
Here's my simplified version of my own strtohex function:
[code]
char *StrToHex(char *input) {
char sdfg[4], outputbuf[2048];
for (unsigned int i = 0; i != strlen(input); i++) {
sprintf(sdfg, "%02x ", input[i]);
strcpy(outputbuf + (i * 3), sdfg);
}
void *asdf = malloc(sizeof(outputbuf));
strcpy((char *)asdf, (const char *)outputbuf);
return (char *)asdf;
}
[/code]
Much cleaner. Even though the previous version may have looked messier, two of the lines were just for debugging, and the other two i didn't need. With that fixed, you can see it's just as good if not better then l2k-shadow's version. (because it doesn't call another function like strcat() and instead uses simple pointer arithmetic to concatinate the previous string to the other.)
July 24, 2007, 2:55 AM
l2k-Shadow
I am deeply sorry that my coding style does not appeal to you when you're using strcpy to copy a WORD, while also using it to copy a buffer which has null characters within it. ever hear of C strings being in ANSI and terminated by a null character? I also generally dislike coding functions which return a pointer due to memory allocation/deallocation complications that i may encounter, but that is just my preference.

also can i see that research on the speed of a JGE/JNL instruciton vs JE/JNZ instruction, please?
July 24, 2007, 3:33 AM
BreW
[quote author=l2k-Shadow link=topic=16901.msg171171#msg171171 date=1185248011]
I am deeply sorry that my coding style does not appeal to you when you're using strcpy to copy a WORD, while also using it to copy a buffer which has null characters within it. ever hear of C strings being in ANSI and terminated by a null character? I also generally dislike coding functions which return a pointer due to memory allocation/deallocation complications that i may encounter, but that is just my preference.
[/quote]
You still didn't answer me on how you would write that. (copying a WORD to a buffer)
And uh..... last time I've checked, strings in C are null terminated char arrays. Pointer arithmetic and the functions that we have posted here proves me right. And I only stated that your version looks less complex because you did not return a pointer, trying to, eh.... i'm not sure how to put this, make me look stupid.

[quote]
also can i see that research on the speed of a JGE/JNL instruciton vs JE/JNZ instruction, please?
[/quote]

Sure. Not entirely sure why you're asking me to compare JGE (instead of JG), but O.K., here:
[code]
void main() {
int i = 500000000;
int tick = GetTickCount();
while(i--) {
if (i != 500) {
}
}
cout << (GetTickCount() - tick) << "\n";

i = 500000000;
tick = GetTickCount();
while(i--) {
if (i > 500) {
}
}
cout << (GetTickCount() - tick) << "\n";
}

tested with that, the results are 1672 and 1688ms, respectively.
[/code]
instruction* by the way.
July 24, 2007, 4:39 AM
l2k-Shadow
[code]
void main()
{
LARGE_INTEGER count, count2;
int j = 1024;
int i = 0;
QueryPerformanceCounter(&count);

for (i = 0; i != j; i++)
{

}

QueryPerformanceCounter(&count2);

printf("%u\n", (count2.QuadPart - count.QuadPart));

QueryPerformanceCounter(&count);

for (i = 0; i < j; i++)
{

}
QueryPerformanceCounter(&count2);

printf("%u\n", (count2.QuadPart - count.QuadPart));


}
[/code]

that's quite odd because my test yielded enough results that when averaged it showed that at some parts one executed faster than the other and vice versa, so i think it depends on how much work your processor has to do at the time of each loop.

EDIT:
i don't see how you got those results btw, using GetTickCount() yields 0 for me (yes i copy+pasted your code into compiler), so your processor must be a pile of crap.

EDIT AGAIN: oh i see you compiled it as debug, that's great for benchmarking speed, i must congratulate you on that one. also if in debug both of those yielded the same results usually or either one or the other was slightly off.
July 24, 2007, 4:48 AM
BreW
[quote author=l2k-Shadow link=topic=16901.msg171174#msg171174 date=1185252502]
that's quite odd because my test yielded enough results that when averaged it showed that at some parts one executed faster than the other and vice versa, so i think it depends on how much work your processor has to do at the time of each loop.
[/quote]
Maybe it's not accurate at how fast those instructions actually are, but it's a good enough indicator to compare the speed of two opcodes.
[quote]
EDIT:
i don't see how you got those results btw, using GetTickCount() yields 0 for me (yes i copy+pasted your code into compiler), so your processor must be a pile of crap.
[/quote]
GetTickCount() yields 0 for you? Wow, your kernel32.dll must be busted. My processor isn't a piece of crap-- It's an AMD Athlon 64 3500+ 2.2 GHz.

[quote]
EDIT AGAIN: oh i see you compiled it as debug, that's great for benchmarking speed, i must congratulate you on that one. also if in debug both of those yielded the same results usually or either one or the other was slightly off.
[/quote]
I did, I just pressed build >> execute sdgsdg.exe. I know, it's great for benchmarking speed isn't it?
You know, it probably is completely inaccurate because there are such things as compiler optimizations. That function effectively does nothing, and the compiler probably edited it so that it DOES do nothing. So there you have it, "release" sure is great for benchmarking speed.  ::) That also explains why your result was 0 for both.
July 24, 2007, 2:44 PM
l2k-Shadow
no it's not good enough. debug compiles extra shit into your code which you didn't code hence the benchmark is tampered with.

Is this good enough for you? I still receive the exact same results:

[code]
void main()
{
LARGE_INTEGER count, count2;
count.QuadPart = 0;
count2.QuadPart = 0;
int j = 128;
int i = 0;
char out[128];

for (int l = 1; l <= 10; l++)
{

QueryPerformanceCounter(&count);

for (i = 0; i != j; i++)
{
memset(out + i, i + 32, 1);
}

QueryPerformanceCounter(&count2);

printf("!= Loop: %u %s\n", (count2.QuadPart - count.QuadPart), out);

QueryPerformanceCounter(&count);

for (i = 0; i < j; i++)
{
memset(out + i, i + 32, 1);
}

QueryPerformanceCounter(&count2);

printf("< Loop: %u %s\n", (count2.QuadPart - count.QuadPart), out);

}
}
[/code]

ASM:
[img]http://www.instimul.com/fjaros/a.png[/img]

and using gettickcount is very bad because its such a low resolution timer.


July 24, 2007, 4:19 PM
K
You guys are cute, kinda like an old married couple.
July 24, 2007, 4:48 PM
BreW
[quote author=l2k-Shadow link=topic=16901.msg171184#msg171184 date=1185293944]
no it's not good enough. debug compiles extra shit into your code which you didn't code hence the benchmark is tampered with.

Is this good enough for you? I still receive the exact same results:

[code]

....

[/code]
[/quote]
Sure that's good enough-- To be honest i couldn't care less, I'm not trying to prove something retarded like that. If you want to waste your time and effort on "proving brew wrong", go ahead. I really couldn't care less what's faster on your processor.

[quote]
and using gettickcount is very bad because its such a low resolution timer.
[/quote]
It's faster and easier to use, especially when something doesn't have to be very accurate. This was just a comparison. You could counteract gettickcount's low resolution by making the amount of loops higher...
I don't really get what you're trying to prove.
July 25, 2007, 12:00 AM
l2k-Shadow
[quote]
i've noticed you're using "i < strlen(in)", which really should be avoided unless completely necessary-- it takes less cpu cycles to check if a variable is not equal to another rather then to check if it's greater then a specified value, where my way would indeed be much more efficient, and let's face it.
[/quote]

i don't want you telling me that your way is much more efficient and getting away with it.
July 25, 2007, 12:05 AM
BreW
[quote author=l2k-Shadow link=topic=16901.msg171195#msg171195 date=1185321946]
[quote]
i've noticed you're using "i < strlen(in)", which really should be avoided unless completely necessary-- it takes less cpu cycles to check if a variable is not equal to another rather then to check if it's greater then a specified value, where my way would indeed be much more efficient, and let's face it.
[/quote]

i don't want you telling me that your way is much more efficient and getting away with it.
[/quote]
Common sense also tells you that JE and JNE are faster then your JG/JL and JGE/JLE, not only results from some test.
July 25, 2007, 3:21 AM
l2k-Shadow
but the results from the test were that during multiple trials, they ended up being equal.
July 25, 2007, 4:44 AM
Zakath
If you really feel like getting technical, why not do the various instructions out in logic gates and do a real speed comparison?

Seriously, this bickering is totally pointless. Start doing something productive or this topic will be locked.
July 25, 2007, 6:53 AM
BreW
[quote]
Hey, does anyone know of an atoi()-like function except is able to convert hexidecimal ? That'd be useful.
[/quote]

Just to let everyone know, I came across a function which was exactly what i was looking for-- sscanf();.
July 25, 2007, 7:07 PM
Yegg
[code]
/home/yegg> printf("%x", atoi ("123"))
7b
[/code]

or

[code]
/home/yegg> printf("0x%x", atoi ("123"))
0x7b
[/code]
July 27, 2007, 7:51 AM
BreW
I ment hexidecimal input... like converting "35" to "5".
July 27, 2007, 1:39 PM
Barabajagal
how does 35 become 5?
July 27, 2007, 6:17 PM
rabbit
35 is ASCII of character 5, IIRC.
July 27, 2007, 6:37 PM
Barabajagal
that's not hexadecimal :(
July 27, 2007, 6:48 PM
rabbit
[quote author=What's wrong with 現のさざ波? link=topic=16901.msg171230#msg171230 date=1185562104]
that's not hexadecimal :(
[/quote]He's brew?
July 27, 2007, 9:02 PM
Yegg
[code]
/home/yegg> (char)0x35
5
[/code]

That's how 35 becomes 5. He just wrote it wrong.
July 28, 2007, 2:07 AM
BreW
[quote author=What's wrong with 現のさざ波? link=topic=16901.msg171230#msg171230 date=1185562104]
that's not hexadecimal :(
[/quote]
Yes it is. I'm so sorry your brain automatically assumes a number is decimal, especially when I just asked about converting string literals of hexidecimal numbers to a value.
July 28, 2007, 5:42 AM
Barabajagal
I'm so sorry you're stupid. </flame>
July 28, 2007, 9:05 PM
BreW
[quote author=What's wrong with 現のさざ波? link=topic=16901.msg171246#msg171246 date=1185656740]
I'm so sorry you're stupid. </flame>
[/quote]
that's okay, apology accepted mr. 0.4.
July 29, 2007, 6:03 PM
Barabajagal
;) it took a real lack of willpower
July 29, 2007, 7:03 PM
Kp
brew: never use sprintf.  It is deprecated specifically because so many people get it wrong.  Use snprintf (or, if you are stuck on Windows, _snprintf, but watch out for the different semantics from a proper snprintf).

The return type of malloc is void* because you can cast the result to any type, as documented in the manual page.  If you are using C++, you would be better served with using new / new[] so that you get the right type automatically.  For beginners, it is best to minimize the number of casts.  As you saw, they can override useful warnings and even make incorrect code build.

Avoid functions which allocate memory and then return a pointer to it.  Although they can be used correctly, many people will use them incorrectly, leading to memory leaks.  UserLoser already recommended a construct that would leak memory.  (Note: sometimes, this method is the most expedient way to solve a problem.  Functions which return an allocated structure are also necessary for certain design patterns.  In such cases, be sure to tag the function with __attribute__((warn_unused_result)).)

With regard to the efficiency of comparison: recall that on x86 processors, the jump instruction is not the instruction which computes the relation of the operands.  That is done by a prior instruction.  The jump instruction reads the eflags register to get the results of the comparison, and alters code flow accordingly.  Also, while there is technically some value in having the most efficient code possible, the speed with which a jump instruction can be processed makes the distinction between the various versions of jump largely irrelevant.  You have spent more time arguing the point than will ever be regained by having chosen the faster one (whichever one is faster, if they even have a speed difference).
July 30, 2007, 1:22 AM

Search