Vallery
Vallery

Reputation: 141

Appending a char to a char* in C?

I'm trying to make a quick function that gets a word/argument in a string by its number:

char* arg(char* S, int Num) {
    char* Return = "";
    int Spaces = 0;
    int i = 0;
    for (i; i<strlen(S); i++) {
        if (S[i] == ' ') {
            Spaces++;
        }
        else if (Spaces == Num) {
            //Want to append S[i] to Return here.
        }
        else if (Spaces > Num) {
            return Return;
        }
    }
    printf("%s-\n", Return);
    return Return;
}

I can't find a way to put the characters into Return. I have found lots of posts that suggest strcat() or tricks with pointers, but every one segfaults. I've also seen people saying that malloc() should be used, but I'm not sure of how I'd used it in a loop like this.

Upvotes: 1

Views: 603

Answers (5)

BLUEPIXY
BLUEPIXY

Reputation: 40145

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *arg(const char *S, unsigned int Num) {
    char *Return = "";
    const char *top, *p;
    unsigned int Spaces = 0;
    int i = 0;

    Return=(char*)malloc(sizeof(char));
    *Return = '\0';
    if(S == NULL || *S=='\0') return Return;
    p=top=S;
    while(Spaces != Num){
        if(NULL!=(p=strchr(top, ' '))){
            ++Spaces;
            top=++p;
        } else {
            break;
        }
    }
    if(Spaces < Num) return Return;
    if(NULL!=(p=strchr(top, ' '))){
        int len = p - top;
        Return=(char*)realloc(Return, sizeof(char)*(len+1));
        strncpy(Return, top, len);
        Return[len]='\0';
    } else {
        free(Return);
        Return=strdup(top);
    }
    //printf("%s-\n", Return);
    return Return;
}

int main(){
    char *word;

    word=arg("make a quick function", 2);//quick
    printf("\"%s\"\n", word);

    free(word);
    return 0;
}

Upvotes: 0

netcoder
netcoder

Reputation: 67695

In your code, when the function returns, then Return will be gone as well, so this behavior is undefined. It might work, but you should never rely on it.

Typically in C, you'd want to pass the "return" string as an argument instead, so that you don't have to free it all the time. Both require a local variable on the caller's side, but malloc'ing it will require an additional call to free the allocated memory and is also more expensive than simply passing a pointer to a local variable.

As for appending to the string, just use array notation (keep track of the current char/index) and don't forget to add a null character at the end.

Example:

int arg(char* ptr, char* S, int Num) {
    int i, Spaces = 0, cur = 0;
    for (i=0; i<strlen(S); i++) {
        if (S[i] == ' ') {
            Spaces++;
        }
        else if (Spaces == Num) {
            ptr[cur++] = S[i]; // append char
        }
        else if (Spaces > Num) {
            ptr[cur] = '\0';   // insert null char
            return 0;          // returns 0 on success
        }
    }

    ptr[cur] = '\0';           // insert null char
    return (cur > 0 ? 0 : -1); // returns 0 on success, -1 on error
}

Then invoke it like so:

char myArg[50];
if (arg(myArg, "this is an example", 3) == 0) {
    printf("arg is %s\n", myArg);
} else {
    // arg not found
}

Just make sure you don't overflow ptr (e.g.: by passing its size and adding a check in the function).

There are numbers of ways you could improve your code, but let's just start by making it meet the standard. ;-)

P.S.: Don't malloc unless you need to. And in that case you don't.

Upvotes: 1

argentage
argentage

Reputation: 2778

You can't assign anything to a string literal such as "".

You may want to use your loop to determine the offsets of the start of the word in your string that you're looking for. Then find its length by continuing through the string until you encounter the end or another space. Then, you can malloc an array of chars with size equal to the size of the offset+1 (For the null terminator.) Finally, copy the substring into this new buffer and return it.

Also, as mentioned above, you may want to remove the strlen call from the loop - most compilers will optimize it out but it is indeed a linear operation for every character in the array, making the loop O(n**2).

Upvotes: 0

vanza
vanza

Reputation: 9903

I will not claim to understand what it is that you're trying to do, but your code has two problems:

  • You're assigning a read-only string to Return; that string will be in your binary's data section, which is read-only, and if you try to modify it you will get a segfault.
  • Your for loop is O(n^2), because strlen() is O(n)

There are several different ways of solving the "how to return a string" problem. You can, for example:

  • Use malloc() / calloc() to allocate a new string, as has been suggested
  • Use asprintf(), which is similar but gives you formatting if you need
  • Pass an output string (and its maximum size) as a parameter to the function

The first two require the calling function to free() the returned value. The third allows the caller to decide how to allocate the string (stack or heap), but requires some sort of contract about the minumum size needed for the output string.

Upvotes: 2

Aftnix
Aftnix

Reputation: 4589

char * Return;   //by the way horrible name for a variable.
Return = malloc(<some size>);
......
......
*(Return + index) = *(S+i); 

Upvotes: 0

Related Questions