The program needs to do the following：

1. initialise the combined string variable.
2. read a string (input1) of no more than 256 characters then: Until the first character of the read string is '#'
3. allocate enough space for a new combined string variable to hold the current combined string variable and the new read-string (input1).
4. then copy the contents of the old combined variable to the new, bigger, combined variable string.
5. then concatenate the newly read-string (input1) to the end of the new combined string.
6. deallocate the old combined string.
7. read a string into input 1 again.
8. After the user has typed a string with '#' print out the new combined string.

test input: where I am #here

expected output: whereIam

actually output: Segmentation fault (core dumped)

Note that the spaces above separate the strings. Write two more test cases.

And that is my own code:

#include<stdio.h>
#include<stdlib.h> // for malloc
#include<string.h> // for string funs

int main(void) {
    char input1[257];
    printf("Please enter a string here: ");
    scanf("%6s",input1);
    int input1_index = 0;
    while (input1[input1_index] != '#') {
        input1_index  ;
    }
    char *combined;
    combined = malloc(sizeof(char)*(input1_index 1 1));
    combined[0] = '\0';
    strcpy(combined,input1);

    printf("%s\n",combined);


    return 0;
}

How to modify my code? What is the Segmentation fault (core dumped)? Thank you all.

CodePudding user response：

With scanf, the specifier %Ns consumes and discards all leading whitespace before reading non-whitespace characters - stopping when trailing whitespace is encountered or N non-whitespace characters are read.

With the input

where I am #here

this

char input1[257];
scanf("%6s", input1);

will result in the input1 buffer containing the string "where".

This while loop

while (input1[input1_index] != '#') {
    input1_index  ;
}

does not handle the specified "[read strings...] Until the first character of the read string is '#'". It simply searches the one string you have read for the '#' character, and can easily exceed the valid indices of the string if one does not exist (as in the case of "where"). This will lead to Undefined Behaviour, and is surely the cause of your SIGSEGV.

---

Your code does not follow the instructions detailed.

A loop is required to read two or more whitespace delimited strings using scanf and %s. This loop should end when scanf fails, or the the first character of the read string is '#'.

Inside this loop you should get the string length of the read string (strlen) and add that to the existing string length. You should allocate a new buffer of this length plus one (malloc), and exit the loop if this allocation fails.

• You should then copy the existing string to this new buffer (strcpy).
• You should then concatenate the read string to this new buffer (strcat).
• You should then deallocate the existing string buffer (free).
• You should then copy the pointer value of the new string buffer to the existing string buffer variable (=).

Then repeat the loop.

In pseudocode, this roughly looks like:

input := static [257]
combined := allocate [1] as ""
size := 0

exit program if combined is null

print "Enter strings: "

while read input does not begin with '#' do
    add length of input to size

    temporary := allocate [size   1]

    stop loop if temporary is null

    copy combined to temporary
    concatenate input to temporary

    deallocate combined
    combined := temporary
end

print combined
deallocate combined

CodePudding user response：

Others have already explained what you should do. But my additional suggestion is to take those detailed instructions and make them comments. Then for each one of them translate into C code:

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

int main(void) 
{
    // initialise the combined string variable.
    char *combined = malloc(1);
    combined[0] = 0;
    while (true) {
        // read a string (input1) of no more than 256 characters 
        char input1[257];
        if (scanf("%6s", input1) != 1) {
            break;
        }
        // then: Until the first character of the read string is '#'
        if (input1[0] == '#') {
            break;
        }
        // allocate enough space for a new combined string variable to hold the current combined string variable and the new read-string (input1).
        char *tmp = malloc(strlen(combined)   strlen(input1)   1);
        // then copy the contents of the old combined variable to the new, bigger, combined variable string.
        strcpy(tmp, combined);
        // then concatenate the newly read-string (input1) to the end of the new combined string.
        strcat(tmp, input1);
        // deallocate the old combined string.
        free(combined);
        combined = tmp; // <-- My interpretation of what the aim is
        // read a string into input 1 again. <-- I guess they mean to loop
    }
    // After the user has typed a string with '#' print out the new combined string.
    printf("%s", combined);
    free(combined);
    return 0;
}

Notice how "initialize", means "make it a proper C string which can be extended later".

When you read anything remember to check if the read was successful.

The first character of input1 is just `input1[0]'. It's always safe to check it (if the read was successful), because it must be the first character or the string terminator. No risk of false detection.

Allocation requires to add the size of combined, input1, and the string terminator.

Copy and concatenation are available in <string.h>.

Notice that the instructions are missing a line stating that the new combined string should become the current combined string, but it's pretty obvious from the intent.

I strongly dislike the suggested (implied) implementation which calls for a sequence such as:

read
while (check) {
    do_stuff
    read
}

My preference goes to the non repeated read:

while (true) {
    read
    if (!check) break;
    do_stuff
}

Finally, free your memory. Always. Don't be lazy!

Just for completeness, another option is to store the size of your combined string, to avoid calling strlen on something you already know. You could also leverage the calloc and realloc functions, which have been available since a lot of time:

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

int main(void) 
{
    size_t capacity = 1;
    char *combined = calloc(capacity, 1);
    while (true) {
        char input1[257];
        if (scanf("%6s", input1) != 1 || input1[0] == '#') {
            break;
        }
        capacity  = strlen(input1);
        combined = realloc(combined, capacity);
        strcat(combined, input1);
    }
    printf("%s", combined);
    free(combined);
    return 0;
}

Finally for the additional test strings:

these are not the droids you're looking for #dummy
a# b## c### d#### e##### #f###### u#######(explicit)

Useless addition

You could also limit the number of reallocations by using a size/capacity couple and doubling the capacity each time you need more. Also (not particularly useful here) checking the return value of memory allocation function is a must. My checking for realloc is needlessly complicated here, because memory is freed at program termination, but, nevertheless, free your memory. Always. Don't be lazy!