1

I'm working on a class project that would require me to make unique strings and I want to concatenate a number to a string. However I do NOT have access to C Standard Library (memset, malloc, etc.). I made this which works:

char* concat(char* name, int num) {
  int i, j;
  char newName[50], stack[5];

  for(i=0; name[i]!='\0'; ++i) {
    newName[i] = name[i];
  }

  for (j=0; num>=1 || num==0; j++) {
    stack[j] = (num % 10) + '0';
    num = num / 10;
    if (num==0) break;
  }

  while (j>=0) {
    newName[i++] = stack[j--];
  }
  name[0] = '\0';

  return newName;
}

But then as I tested it with multiple strings, I realized that newName was being reused over and over. For ex. This test file outputs the following:

int main() {

  char* rebecca = concat("rebecca", 1);
  char* bill = concat("bill", 2);

  Write(rebecca);  /* bill2ca1 */
  Write(bill);     /* bill2ca1 */
}

It successfully appends the 1 to rebecca, but then when I call concat on bill, it overwrites the first 5 letter but keeps the same chars from before in newName. QUESTION: How to clear a char array so the next time it's called it will be set to empty, or dynamically allocate it (without using C Standard Library)?

TheJKFever
  • 685
  • 7
  • 25
  • 2
    Function *declarations* go in header files; function *definitions* go in C files. – Jonathon Reinhart Nov 19 '15 at 14:38
  • 2
    Step through your code in a debugger, carefully watching your string in memory, to see where you're going wrong. Stack Overflow isn't here to debug your code for you. – Jonathon Reinhart Nov 19 '15 at 14:39
  • The question is how to clear a char array without using standard library memset, bzero, or strcpy. I'm not trying to find where it's going wrong, I wrote in the post where it's going wrong, it's reusing the same array. I'm asking how to clear the array. An appropriate question that after searching and only finding the above 3 methods of arr[0] = '\0' which none of which worked, StackOverflow is a valid place to ask. – TheJKFever Nov 19 '15 at 14:53
  • 1
    Also I'm familiar with how header vs c files work, this is a utility header file that is externing functions for other files. in my special case, it makes sense for me to use this implementation. That's very beside the question though. Please answer the question @JonathonReinhart – TheJKFever Nov 19 '15 at 14:55
  • @Sourav Ghosh Reason for re-opening? This is the third time today someone asks why their program is crashing because they return pointers to local variables. If you have a better canonical duplicate, feel free to share it. – Lundin Nov 19 '15 at 15:12
  • See: [Pointer to local variable](http://stackoverflow.com/questions/4570366/pointer-to-local-variable), [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope). – Lundin Nov 19 '15 at 15:21
  • @Lundin Sir, I understand your point, but what OP is actually looking for is to clear an array without `memset()`. The fact that he sees the behaviour is no doubt because of the reason you mentioned, but a dupe of that will not be exactly appropriate, I feel. Thank you for adding the link in the comment. :) – Sourav Ghosh Nov 19 '15 at 15:45

3 Answers3

1

Without using malloc, you can simply put the memory on the stack of the calling function, to keep in the scope where it is needed. It's easier to add the buffer pointer to the argument list like so:

char* concat(char *newName, char* name, int num) {
  int i, j;
  char stack[5];
   :
   :
}

int main() {
  char rebecca[50];
  char bill[50];

  concat(rebecca, "rebecca", 1);
  concat(bill, "bill", 2);

  write(rebecca);
  write(bill);
}

Generally speaking, assign memory where it will be used. Embedded programming (which might need to run for months without a reboot) avoids malloc like the plague, just because of the risk of memory leaks. You then need to assign extra space since you may not know the size at compile time, and then ideally check for running past the end of the buffer. Here we know the string sizes and 50 chars is more than enough.

Edit: The other issue is that you're not null terminating. The print will go until it hits 0x00. Your line

name[0] = '\0';

should be

newName[i] = '\0';
Nerf Herder
  • 416
  • 3
  • 11
  • Just a remark: I would say leaks are only part of the reasons why `malloc()` is frowned upon or outright forbidden in embedded systems. Perhaps more important is the more complex dynamic behavior both in terms of time and memory. If you can pre-allocate statically and cover all expected data you are much safer. (If not, you must guard against that anyway.) Another issue is fragmentation with certain use cases or bad allocators. – Peter - Reinstate Monica Nov 19 '15 at 16:04
0

You've got a major issue that you're overlooking. In your function, newName is a local variable (array) and you're returning it from the function. This invokes undefined behavior. The beauty of UB is that, sometime it appears to work as expected.

You need to take a pointer and allocate memory dynamically instead, if you want to return it from your concat() function. Also, in the main(), after using it, you need to free() it.

A better alternative, maybe, if you choose to do so, is

  1. Define the array in the caller.
  2. Pass the array to the function.
  3. Inside the function, memset() the array before you perform any other operation.

One thing to remember, this way, every call to the function will clean the previous result.


EDIT:

If you cannot use memset(), in the main, you can use a for loop like

for (i = 0; i < sizeof(arr)/sizeof(arr[0]); i++)
    arr[i] = 0;

to clear the array before passing it on next time.

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • doing something like memset would be ideal, unfortunately I'm not able to include standard libraries. Is there a way I can do this without memset itself, such as looping through newName and setting each index to '\0' ? – TheJKFever Nov 19 '15 at 14:59
0

You're returning the address of a local variable. Since the variable goes out of scope when the function returns, this invokes undefined behavior.

You function should dynamically allocate memory for the result of the concatenation, then return that buffer. You'll need to be sure to free that buffer later to prevent a memory leak:

char* concat(char* name, int num) {
  int i, j;
  char *newName, stack[5];
  // allocate enough space for the existing string and digits for a 64-bit number
  newName = malloc(strlen(name) + 30);

  for(i=0; name[i]!='\0'; ++i) {
    newName[i] = name[i];
  }

  for (j=0; num>=1 || num==0; j++) {
    stack[j] = (num % 10) + '0';
    num = num / 10;
    if (num==0) break;
  }

  while (j>=0) {
    newName[i++] = stack[j--];
  }
  newName[i] = '\0';

  return newName;
}

int main() {

  char* rebecca = concat("rebecca", 1);
  char* bill = concat("bill", 2);

  Write(rebecca);
  Write(bill);

  free(rebecca);
  free(bill);
}
dbush
  • 205,898
  • 23
  • 218
  • 273
  • ah this would be great if I could use malloc. But I can't use any of the standard library. Any suggestion of how to dynamically allocate without using malloc? – TheJKFever Nov 19 '15 at 15:02
  • 1
    This isn't good advise, since proper program design dictates that the module doing the allocation is also the one responsible for cleaning up its own mess. Functions based on "someone else will clean this up" _will_ create memory leaks and such program designs cost astronomic amounts of money for the software industry as whole, so please don't teach them on the internet. – Lundin Nov 19 '15 at 15:09
  • @Lundin (1) While "keep malloc/free in the same module" is a sensible guideline it's certainly not absolute. In fact, some uses of malloc are precisely to create an object which will be freed some other time some other place, or possibly never. (2) This `concat` has the same rationale as `strdup()`. (3) The module calling `concat` *is* the one cleaning up, exactly as if they had called malloc directly. – Peter - Reinstate Monica Nov 19 '15 at 15:39