Jump to content
The Dark Mod Forums

[c++] What Is The Point Of This Function?


OrbWeaver

Recommended Posts

I found this gem in DarkRadiant - I think its purpose is to remove null entries from argv[], but why they bothered shifting stuff around when they could just copy the valid items into a new array I have no idea.

 

void args_init(int argc, char* argv[])
{
 int i, j, k;
 for (i = 1; i < argc; i++)
 {
for (k = i; k < argc; k++)
  if (argv[k] != 0)
	break;
if (k > i)
{
  k -= i;
  for (j = i + k; j < argc; j++)
	argv[j-k] = argv[j];
  argc -= k;
}
 }
 g_argc = argc;
 g_argv = argv;
}

 

I would have done it like this:

 

void args_init(int argc, char* argv[]) {
  int inPtr, outPtr;
  char* outArgv[argc];

  for (inPtr = outPtr = 0; inPtr < argc; inPtr++) {
  if (argv[inPtr] == 0) // null pointer
	 break;
  else
	 outArgv[outPtr++] = argv[inPtr];
  }

  g_argc = outPtr;
  g_argv = outArgv;
}

 

Maybe some experienced programmer can point out why my version is crap, but it seems to me to do the same thing with more readability and fewer lines of code.

Link to comment
Share on other sites

void args_init(int argc, char* argv[])
{
 int i, j, k;
 for (i = 1; i < argc; i++)
 {
for (k = i; k < argc; k++)
  if (argv[k] != 0)
	break;
if (k > i)
{
  k -= i;
  for (j = i + k; j < argc; j++)
	argv[j-k] = argv[j];
  argc -= k;
}
 }
 g_argc = argc;
 g_argv = argv;
}

 

Maybe some experienced programmer can point out why my version is crap, but it seems to me to do the same thing with more readability and fewer lines of code.

 

IMO this function is pretty dangerous. The value of k is undefined after the first for loop, because it depends on the compiler what value k has. It can be 'i', it can be 'i+1' or it can be some totally different value. You might already see different behaviours compiling this on Visual Studio 6 or gcc, so I would suggest rewriting it. According to the C standard the loopvariable is not defined when the loop ended and it is up to the compiler developer to what it contains.

Gerhard

Link to comment
Share on other sites

IMO this function is pretty dangerous. The value of k is undefined after the first for loop, because it depends on the compiler what value k has. It can be 'i', it can be 'i+1' or it can be some totally different value.

 

Good point, I didn't notice that. k is only defined within the for statement so it may not even be visible after the loop has finished.

 

You might already see different behaviours compiling this on Visual Studio 6 or gcc, so I would suggest rewriting it.

 

Yeah, I think I'll replace it. I don't understand why this is even necessary since argv[] shouldn't contain any null pointers.

 

Incidentally I am in the process of upgrading the build scripts so that DarkRadiant can be built with GCC on both Windows and Linux. Most of the work is done except for the OpenGL libraries, which require some more investigation since libGL is not available under Windows.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

  • Recent Status Updates

    • The Black Arrow

      Hope everyone has the blessing of undying motivation for "The Dark Mod 15th Anniversary Contest". Can't wait to see the many magnificent missions you all may have planned. Good luck, with an Ace!
      · 0 replies
    • Ansome

      Finally got my PC back from the shop after my SSD got corrupted a week ago and damaged my motherboard. Scary stuff, but thank goodness it happened right after two months of FM development instead of wiping all my work before I could release it. New SSD, repaired Motherboard and BIOS, and we're ready to start working on my second FM with some added version control in the cloud just to be safe!
      · 2 replies
    • Petike the Taffer  »  DeTeEff

      I've updated the articles for your FMs and your author category at the wiki. Your newer nickname (DeTeEff) now comes first, and the one in parentheses is your older nickname (Fieldmedic). Just to avoid confusing people who played your FMs years ago and remember your older nickname. I've added a wiki article for your latest FM, Who Watches the Watcher?, as part of my current updating efforts. Unless I overlooked something, you have five different FMs so far.
      · 0 replies
    • Petike the Taffer

      I've finally managed to log in to The Dark Mod Wiki. I'm back in the saddle and before the holidays start in full, I'll be adding a few new FM articles and doing other updates. Written in Stone is already done.
      · 4 replies
    • nbohr1more

      TDM 15th Anniversary Contest is now active! Please declare your participation: https://forums.thedarkmod.com/index.php?/topic/22413-the-dark-mod-15th-anniversary-contest-entry-thread/
       
      · 0 replies
×
×
  • Create New...