Returning the length of a 'hidden' string within each of the sentences of another string

Question

The following program is supposed to return the length of a 'hidden' string within each of the sentences of another string, using pointers and avoiding as much as possible the use of [] as a means to access strings. We were instructed on how to declare the functions, hence their declarations are a given.

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

int getHiddenString(const char * str1 , const char * str2);
int divideStringIntoSentences(char * string, char * sentences[]);
char* readLine();
int main()
{
char *ptrarr[20];
int i = 0;
printf ("Please enter a string:\n");
char *str1 = readLine();
printf ("Please enter a hidden string:\n");
char *str2 = readLine();
if (str2 == NULL) return EXIT_FAILURE;
int j = divideStringIntoSentences(str1, ptrarr);
for (; i <= j; i++)
printf ("Sentence %d contains hidden string prefix length %d\n", i,   
    getHiddenString(*(ptrarr + i), str2));
free (str1);
free (str2);
return 0;
}

int getHiddenString(const char * str1 , const char * str2)
{
int hidlen = 0;
for (; *str1 != '\0' && *str2 != '\0'; str1++)
if (*str1 == *str2)
{
    hidlen++;
    str2++;
}
return (hidlen);
}

char* readLine()
{
int capacity = 1, index = 0;
char c;
char *str1 = (char*) malloc(capacity * sizeof(char));
if (str1 == NULL) return NULL;
for (c = getchar(); c != '\n' && capacity <= 100; c = getchar())
{
    if (index == capacity - 1)
    {
        str1 = (char*) realloc(str1, (++capacity) * 2 * sizeof(char));
        if (str1 == NULL) return NULL;
        capacity*=2;
    }
    *(str1 + index++) = c;
}
*(str1 + index) = '\0';
return str1;
}

int divideStringIntoSentences(char * string, char * sentences[])
{
int sentnum = 0;
*sentences = string;
for (; *string != '\0'; string++)
if (*string == '.')
{
    *string = '\0';
    *(++sentences) = (string + 1);
    sentnum++;
}
return (sentnum);   
}

Answer 1

Some generals comments:

Define functions in reverse order to avoid prototypes, make local functions static, add void to empty parameter lists, use braces even when unnecessary, use spaces consistently around operators, define a single variable per line, return statements don't need brackets (you are inconsistent in this).

In function readLine

This function has many issues, some minor, such as casting malloc, using sizeof(char) (sizeof char is 1 by definition) and the arbitrary limit on a new line of 100 chars. In your variable name, str1, the '1' is unnecessary, as you only use one string. And your default sentence allocation of 1 char is silly - start with something reasonable, say 64 (arbitrary value).

More seriously, getchar returns an int not char. This allows it to hold the value for EOF, the end-of-file marker. Your for-loop should test for EOF. If you fail to do this then if you input from a redirected file, the program will fail if EOF is reached without finding a \n. A better loop would be, for example:

int ch;
while((ch = getchar()) != EOF) {
    if (ch == '\n') {
        break;
    }
    // do the necessary stuff
}

Your reallocation should compute the new capacity then use that in the realloc call rather than computing it twice. And your realloc call as it currently stands leaks memory on failure - the original string str1 is not freed by realloc on failure.

In function divideStringIntoSentences

You need to pass the size of sentences[] into the function to allow for overflow detection.

In function getHiddenString

Parameter names of sentence and hidden might be more understandable than str1, str2. More seriously, I don't know what it is supposed to do. If it is supposed to find the hidden string in the sentence, then it doesn't work. Library function strstr would do the job for you.

In main

Although the functions handle malloc failures, main does not check for str1 in the returned values. Again str1 and str2 are badly named, as is ptrarr. The for-loop should define the loop variable and should loop up to but not including j: for (int i = 0; i < j; ++i). And the printout "Sentence # contains hidden string prefix length #" is printed even if the sentence doesn't contain the hidden string. This make me think that the getHiddenString function is telling us the number of consecutive chars from the hidden string contained in the sentence, or some such. In that case it still doesn't work and is badly named (eg if I give it "two twa twe" and the hidden string "twe" it says, "Sentence 0 contains hidden string prefix length 4"

Answer 2

A couple of points:

• I don't know if it's just the way the question is formatted, but better indentation would improve readability
• In divideStringIntoSentences, it would be better to add { } around the if as if you add something to the loop, you might forget to add the { }s
• In divideStringIntoSentences there will be an extra sentence if the string ends with a full stop
• You've got a problem if there are more than 20 sentences.