last day (19 days later) » 

6:53 AM
0
Q: Setting NULL causes lag with [MAX_STRING_LENGTH] = {'\0'};

Fred EllisI thought it would be a good best practice to search thru my code for any references like .. char buf[MAX_STRING_LENGTH]; ... and replace them with ... char buf[MAX_STRING_LENGTH] = {'\0'}; Doing a search in the code I have a number that are set to null (around 239) and others that are no...

 
If these arrays are local to functions, they have to be initialised every time the function is called, over and above what the function does. The entire array is filled with 0 not just the first element.
 
When you initialize an array with an initializer list, you tell the compiler to initialize all of the array elements. Any elements that don't have an explicit initializer (i.e., all the ones except the first) get initialized to 0.
What was the reason for changing the 1116 arrays that were not initialized? If it was just a general sense of good hygiene, don't do that. <g>
 
"and pushed the code live" - Don't you have code reviews? Your co-workers should be able to help you with this, or else the game is Doomed.
 
Interesting. So setting these 1k+ entries to fill up with 0 instead of just the first element might be eating up a lot of memory and causing lag due to resource exhaustion. How do I know when I really 'need' to set to 0?
@TedLyngmo the game has gone thru a number of coders. I am the main active one now.
 
It doesn't sound like there is true "resource exhaustion", just "wasted CPU cycles due to initialization". You could consider using e.g. mmap to allocate arrays with pages zeroed out on demand if your buffers are as large as I suspect they are.
 
6:53 AM
It does not use any extra memory (you declared the array length already) but execution time. If you really want to be tidy, then char buf[MAX_STRING_LENGTH]; buf[0] = 0; will achieve it.
 
@Fred Ellis I see. Put up enough code for someone here to be able to give you some hints. For example, if your char buf[] is going to be filled directly by some function, why waste time initializing it? If you are sure it won't be read in its uninitialized state and it's close to the inner loop of something time critical, don't initialize it.
 
@TedLyngmo with 1k+ entries I would not know which ones to pick as examples. Heh. I ran into a situation where not having the Null set caused problems with the function working so I put them back there specifically. After all like I said originally there were already 200+ set for 'whatever' reason. I'll add the example function to the original post since it is a bit long.
 
If that size macro is any indication of what these are used for, that is frankly a hideous solution (full-zero-out) to what is potentially the real problem: someone, somewhere, isn't terminating their strings with nullchars. If the intent is to create a terminated string default, then do that: char buf[MAX_STRING_LENGTH]; *buf = 0;. If you have some rogue code out there expecting to provide string data that's not terminating the data thereafter, that's a bug in that code. But filling a big memory buffer full of nullchars only to blast over some/most of it with later writes a terrible.
 
Now you know why C doesn't initialize locals by default.
 
@WhozCraig I appreciate the honest feedback. I am actually doing my best to try to debug this code running it thru tools like CPPCheck, etc to tighten up the code and avoid memory leaks.
 
6:53 AM
So this is c++ code ? If so, remove the C tag and note that in your question, lest the C++ purists that confuse the standard library with the actual formal language specification reap a holy war upon you. Frankly, I'm surprised no one has already mentioned you pick a language. Anyway, From my prior comment, strcpy(key1, buf + 1) per the specification of strcpy that terminates the string, So, filling the rest with nullchars is, by definition, a waste of CPU time.
 
@WhozCraig the code was ported from C to C++ so there are elements of both still. I like your idea. I could search on char buf[MAX_STRING_LENGTH]; and replace with the additional details you mentioned of setting *buf = 0; Would it be = 0 though or = {'\0'} ?
 
@FredEllis it's the same. In C '\0' is just an int with value 0. In C++ it's a char with value zero which is also equal to integer 0. It's just that sometimes '\0' expresses that it's used for NULL termination. Note that atoi is bad don't use it
 
Yeah, I feel for you. Good C code and good C++ code to accomplish the same task, using their respective standard libraries, will more-than-likely end up with entirely different approaches to their eventual solutions. Taking a wall of C code and dropping it into a C++ environment isn't ideal, but it sounds like you're more interested in getting function over form at this point, especially if you inherited this behemoth. Regarding your question, either works. I prefer what I showed you, as it prevents warning spam when same-code is compiled with side vs. narrow chars.
 
@FredEllis Yes, it sure looks like something that started out as C code but got some half hearted C++ pasted on top of it later. But even as C code it isn't very maintenance friendly. Who writes if (count < 200) etc. in a function long enough to fill 2 screens and expect anyone to understand what 200 means? I wouldn't understand my own code two weeks later if done like that. If you really care about this project, you could perhaps attack one suspicious function at a time and C++-ify it. Remove magical limits, use vectors and strings. etc.
 
It is definitely a labor of love trying to keep it limping along. Ideally I would love to know more about identifying memory problems but when I research on it things seem to go over my head fairly quickly. I am going to try replacing things like .... char key1 [MAX_STRING_LENGTH] = {'\0'}; .... with char buf[MAX_STRING_LENGTH]; buf = 0; And entries for ... char buf[MAX_STRING_LENGTH]; .. with the same as above and see if I break anything.
 
6:53 AM
Aside: it's not buf = 0; but *buf = 0; or buf[0] = 0;
 
@WeatherVane thanks! Just the kind of bonehead mistake I would make. :)
@phuclv what should I use instead of atoi in the function above?
 
@FredEllis please read my link above and you'll know that the stoX has been introduced to the standard to fix the issues with atoX. So you can use stof, stod for example
 
@phuclv I replaced the instances of atoi with std::stoi .. in the do_olist function above. Seemed to work fine afterwards. Doing a search in the codebase there are 700+ instances of atoi Do you think it would be dangerous to do a find/replace?
 
@FredEllis you tagged both C and C++, which is a no-no on SO. They're very different with different behaviors so choose only one. In C++ there's also std::strtoX. For the most part I guess replacing all works, because the new version will throw exception in cases that are undefined behavior for atoi and you'll catch the bug faster. On C however those functions will set errno, so you don't have any UB but you may get different results. But that's out of the scope of this and you should ask a different one if you want more info
but if you use C++ then you should avoid all those char*, strcpy or strcasestr and use std::string instead. You should also avoid pointers. Iterators would be better
 

  last day (19 days later) »