Sorry, but I don’t believe your diagnosis or C coding.
In the first place the strncpy(temp_path, env_path, strlen(env_path)); will overwrite what’s in the storage pointed to by temp_path, starting at the beginning, so it doesn’t matter what’s in memory before. If it were str(n)cat, then it would be a different story.
In the second place in the call strncpy(temp_path, “”, lenght_env_path + strlen(icon_filename) ); the length argument is useless since the copying will stop after the ‘\0’ in “” has been copied over. You could just have easily have written *temp_path = ‘\0’;
You may be on the right track, but I don’t believe this diagnosis.
The original code does not include the strncpy(temp_path, “”… line. I added it, that’s why it’s in red. Without it, what was in memory before does matter.
The diagnosis was simple: when the memory allocated to temp_path was full of junk, the free(temp_path) operation later in the code caused a segfault.
The fix was simple too: temp_path1 never caused a segfault, because the code contained a line to null it, cleaning whatever junk may have been in it. So I just took the null-ing line used for temp_path1 and also peformed one for temp_path.
I’m not a programmer, so there may be better ways to do things. And despite the patch, I still have questions:
Why does malloc sometimes allocate dirty memory? Shouldn’t it be cleaned before allocation?
Why does the strncpy( temp_path, env_path, strlen(env_path)) not seem to resize temp_path to match env_path? Is there no null terminator on env_path or do I need to do (strlen(env_path)+1)?
Why does just this particular machine have the problem? Is it related to it being a VM?
To answer my own question, I re-compiled using the variable lenght_env_path in the statement, because it includes the +1 already and is used in the temp_path1 null statement. Using the new code, temp_path does indeed resize to match env_path.
I’m just saying that if you submit your patch it will probably be scrutinised by C programmers who will come to the same conclusion as I have, that your added code to “clear the string” really does nothing useful, since strncpy will overwrite whatever was in the string storage anyway, and that if it works, it’s only because you perturbed the code enough to hide the bug which is still lurking somewhere else.
malloc does not guarantee that the memory will be cleared, it says so in the manual page. If you want the memory to be cleared, use calloc.
Your idea that the string can be “resized” indicates that you don’t understand how strings work in C.
In short, I think the problem is deep and hard to find, as most memory corruption problems are. Usually the crash is only experienced a distance away from where the actual bug lies.
Since you have resolved the problem for yourself, and FreeNX doesn’t seem to be maintained, I would just leave it be and not try to submit the patch.
Humm … I have no idea. I install FreeNX in kvm virtual machines running on Linux hosts and on physical Linux servers as well and don’t notice differences … but I never used VMware and always managed to stay away from Windows.
If it’s only affecting one machine and you can not reproduce it, it will be difficult to present it as a bug.