Combine directory and file path - C

And there is a memory leak:

const char *one = combine("foo", "file");
const char *two = combine("bar", "");
//...
free(one);   // needed
free(two);   // disaster!

Edit: Your new code looks better. Some minor stylistic changes:

  • Double semi-colon ;; in line 4.
  • In line 6, replace strlen(path2) == 0 with path2[0] == '\0'' or just !path2[0].
  • Similarly in line 9.
  • Remove loop determining last_char, and use const char last_char = path1[strlen(path1) - 1];
  • Change if(append_directory_separator) to if(last_char != directory_separator[0]). And so you don't need the variable append_directory_separator any more.
  • Have your function also return destination, similar to strcpy(dst, src), which returns dst.

Edit: And your loop for last_char has a bug: it always returns the end of path1, and so you could end up with a double slash // in your answer. (But Unix will treat this as a single slash, unless it is at the start). Anyway, my suggestion fixes this--which I see is quite similar to jdmichal's answer. And I see that you had this correct in your original code (which I admit I only glanced at--it was too complicated for my taste; your new code is much better).

And two more, slightly-more subjective, opinions:

  • I would use stpcpy(), to avoid the inefficiency of strcat(). (Easy to write your own, if need be.)
  • Some people have very strong opinions about strcat() and the like as being unsafe. However, I think your usage here is perfectly fine.

  1. The only time you use last_char is in the comparision to check if the last character is a separator.

Why not replace it with this:

/* Retrieve the last character, and compare it to the directory separator character. */
char directory_separator = '\\';
if (path1[strlen(path1) - 1] == directory_separator)
{
    append_directory_separator = 1;
}

If you want to account for the possibility of multiple character separators, you can use the following. But be sure when allocating the combined string to add strlen(directory_separator) instead of just 1.

/* First part is retrieving the address of the character which is
   strlen(directory_separator) characters back from the end of the path1 string.
   This can then be directly compared with the directory_separator string. */
char* directory_separator = "\\";
if (strcmp(&(path1[strlen(path1) - strlen(directory_separator)]), directory_separator))
{
    append_directory_separator = 1;
}
  1. The less error-prone method would be to have the user give you the destination buffer and its length, much the way strcpy works. This makes it clear that they must manage allocating and freeing the memory.

  2. The process seems decent enough. I think there's just some specifics that can be worked on, mostly with doing things in a clunky way. But you are doing well, in that you can already recognize that happening and ask for help.