C / C++ / MFC

Re: Anything wrong [modified]

31-Mar-11 4:46

<br />
	BYTE message[5];<br />
	message[0] = 15;<br />
	message[1] = 33;<br />
	message[2] = 2;<br />
	message[3] = 23;<br />
	message[4] = 33;<br />
        .<br />
        . <br />
        .<br />
        .<br />
        .<br />
  <br />
	int	messageLengh = 60;<br />
<br />
	char szLogBuff [512]={0};<br />
	for(int i=0;i<messageLengh;i++)<br />
	{<br />
		char chz[4]={0};<br />
		sprintf(chz,"%d ",i);<br />
		strcat(szLogBuff,chz);<br />
	}<br />
        printf(szLogBuff);<br />

Do you find anything wrong with this code?
Do I need to pad the buffer with a '\0'? or just assigning 0 to the buffer initially is fine?

Luc Pattyn31-Mar-11 5:17

31-Mar-11 5:17

Smith# wrote:
Do you find anything wrong with this code?
Do I need to pad the buffer with a '\0'? or just assigning 0 to the buffer initially is fine?

~~yes. no. yes.~~

[MODIFIED]
if it really is the indexes you want to print, then it is: no. no. yes. (but why show all the message stuff then?)
[/MODIFIED]

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum

Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.

modified on Thursday, March 31, 2011 2:26 PM

Smith#31-Mar-11 5:33

31-Mar-11 5:33

The first yes makes me not to click on the x of this window. Could you please tell me what is wrong with the code before I click on x?

Luc Pattyn31-Mar-11 7:18

31-Mar-11 7:18

Smith# wrote:
char chz[4]={0};

this buffer is too small, your sprintf statement may generate a minus sign, three digits, a space, and a NULL, that is 6 characters.

60 of these could take up to 301 chars, so char szLogBuff [512]={0}; is sufficiently large, and as others already pointed out, your total initialization is a waste, as each strcat will move the initial terminating NULL backwards.

BTW: you could get the correct result with less code, and save some cycles, and never have the bug you had, by having sprintf() fill the final buffer right away; all it takes is a variable pointer as the destination, initialized to szLogBuff, and incremented by the return value of sprintf!

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum

Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.

CPallini31-Mar-11 7:37

31-Mar-11 7:37

Luc Pattyn wrote:
your sprintf statement may generate a minus sign

No, it cannot. Poke tongue | ;-P

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke

[My articles]

Luc Pattyn31-Mar-11 7:41

31-Mar-11 7:41

from the code snippet I can't tell what the definition of BYTE is, could be signed, could be unsigned; I prepare for the worst. Anyway, it isn't really relevant, the buffer isn't sufficiently large.

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum

Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.

CPallini31-Mar-11 8:02

31-Mar-11 8:02

You didn't read the snippet, did you?
(hint: it sprints the index, the BYTE part is completely insignificant).
Smile | :)

Luc Pattyn31-Mar-11 8:09

31-Mar-11 8:09

How silly.
So that must be wrong too. Laugh | :laugh:

Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum

Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.

CPallini31-Mar-11 8:17

31-Mar-11 8:17

CPallini31-Mar-11 5:44

31-Mar-11 5:44

The code (albeit not beautiful) is correct.
With szLogBuf[512]={0}; you actually initialize the whole buffer with 0 (that is '\0').
Smile | :)

Smith#31-Mar-11 5:52

31-Mar-11 5:52

Then what is the best way to clear the buffer initially?

Waldermort31-Mar-11 6:14

Waldermort

31-Mar-11 6:14

Since you are dealing with strings there isn't a need to clear the whole buffer.

char x[100] = { 0 };
internally calls memset. In your case it's a waste of (7 or so not including the loop inside memset) CPU cycles.
char x[100];
x[0] = 0;
initially creates an empty string. Just 1 CPU cycle.

But then, calling functions like strcpy and sprintf don't require the buffer to be empty whereas strcat NEEDS a string (empty or otherwise) to append to.

Waldermort

Smith#31-Mar-11 6:22

31-Mar-11 6:22

whereas strcat NEEDS a string (empty or otherwise) to append to.

so I'll have to memset everything to 0 or initialize everything to 0 right?

Waldermort31-Mar-11 6:35

Waldermort

31-Mar-11 6:35

No. When dealing with string buffers you only need to set the first char/wchar/tchar to 0.

char myString[100]; // Is a buffer of 100 chars the a string may be copied into.<br />
strcat( myString, "Hello World!" ); // will fail because the buffer is full of junk.<br />
// but<br />
strcpy( myString, "World!" ); // will work because it doesn't care what is in the buffer<br />
myString[0] = 0; // copies an empty string to the buffer<br />
strcat( myString, "Hello World!" ); // appends the string to the empty string ( also sets the character after '!' to \0 or NULL.

The new string is actually 13 chars because there is a 0 at the end.

On another note. You shouldn't be using those string functions. There are safer versions available:
strcpy_s( myString, 100, "Hello World!" );
These check to make sure the buffer is large enough for the string

Also, you might want to read up on TCHAR and the unicode/ansi string functions

TCHAR myString[100];<br />
_tcscpy_s( myString, 100, _T("Hello World!");

Waldermort

Luc Pattyn31-Mar-11 7:18

31-Mar-11 7:18

CPallini wrote:
The code ... is correct.

Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum

Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.

CPallini31-Mar-11 7:36

31-Mar-11 7:36

It isn't wrong. It isn't elegant nor optimal, but not wrong. I think this is a great achievement, after all.
(It's a subliminal suggestion for you: could you please hire the OP?)

Luc Pattyn31-Mar-11 7:39

31-Mar-11 7:39

CPallini wrote:
It isn't elegant

true.

CPallini wrote:
nor optimal

true.

CPallini wrote:
not wrong

false. see my reply to OP.

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum

Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.

CPallini31-Mar-11 8:02