6

The problem with this code is that it does not actually print anything after the user enters in some text in the command line.

The purpose of the code is to accept the number of lines the user will enter in via command prompt after the file name. Then the user will type something in to reverse. The program is supposed to reverse the user input for each line.

Example Input = the big red dog

Example Output = dog red big the

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#define SIZE 80

char * reverseWords(char *string);

//argc is the count of cmd arguments.
//each command line argument is of type string
int main(int argc, char *argv[]){

    //initialize local variables
    int i;
    int N;
    char str[SIZE];

    for(i = 1; i <argc; i++)
    {
        //set N equal to the users number in the command line 
        N = atoi(argv[i]);
    }

    if(argc != 2){//2 means that something is in the argument.
        printf("ERROR: Please provide an integer greater than or equal to 0");
        exit(1);//exit the program
    }else if(N < 0){//We cant have a negative array size.
        printf("ERROR: Please provide an integer greater than or equal to 0");
        exit(1);//exit the program
    }else{
        for(i = 0; i < N; i++){
            /*
            fgets(pointer to array, max # of chars copied,stdin = input from keyboard) 
            */
            fgets(str,SIZE,stdin);

            printf("%s", reverseWords(str)); //<---does not print anything....
        }           
    }
    return 0;
}   


char * reverseWords(char *line){

    //declare local strings 
    char *temp, *word;
    //instantiate index
    int index = 0;
    int word_len = 0;
    /*set index = to size of user input
        do this by checking if the index of line is
        equal to the null-character.
    */
    for(int i = 0; line[i] != '\0';i++)
    {
        index = i;//index = string length of line.
    }

    //check if index is less than 0.
    //if not we decrement the index value.

    for(index; index != -1; index--){
        //checking for individual words or letters
        if(line[index] == ' ' && word_len > 0){
            strncpy(word,line,word_len);
            strcat(temp , (word + ' '));
            word_len = 0;

        }else if(isalnum(line[index])){
            word_len == word_len+1;
        }//end if

    }//end loop

    //copy over the last word after the loop(if any)
    if(word_len > 0){
        strncpy(word,line,word_len);
        strcat(temp,word);
    }//end if
    line = temp;
    return line;
}//end procedure 
pewpew
  • 700
  • 2
  • 9
  • 32

4 Answers4

6

It should come without surprise that reverseWords prints nothing. Why?

char * reverseWords(char *line){
    ...
    char *temp, *word;
    ...
    line = temp;
    return line;
} //end procedure

Where is line pointing? (to temp). Where was temp declared? (in reverseWords). How much storage was allocated to temp (none -- it is an uninitialized pointer)

Further, what happens to the memory associated with the function reverseWords when it returns? (it's destroyed...), so even if you had done something like char temp[strlen(line)+1] = "";, reverseWords would venture off into Undefined Behavior because the pointer you return, points somewhere within the reverseWords stack frame that was destroyed when reverseWords returned...

How do you fix this? You have three options, (1) pass a second pointer to a second array with sufficient storage, e.g.:

char *revwords (char *rline, char *line)

or, (2) dynamically allocate storage for temp so that the memory associated with temp survives the return of reverseWords, or

(3) use an adequately sized array for temp in reverseWords and overwrite line with the data in temp before the return. (e.g. use strcpy instead of the assignment line = temp;)

While dynamic allocation is straight forward, and creating a separate array in reverseWords is fine, you are probably better passing a second sufficiently sized array as a parameter to reverseWords.

It is completely unclear what you are doing with the argc and argv arguments in your code, the arguments to main have been omitted from the example below. The following is a short example of reversing the words in each line read from stdin,

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

#define SIZE 256

char *revwords (char *rline, char *line);

int main (void) {

    char line[SIZE] = "", rline[SIZE] = ""; /* storage for line/rline */

    while (fgets (line, SIZE, stdin)) { /* for each line on stdin */
        printf ("\n line: %s\nrline: %s\n", line, revwords (rline, line));
        *rline = 0; /* set first char in rline to nul-byte */
    }

    return 0;
}

char *revwords (char *rline, char *line)
{
    size_t lnlen = strlen (line);   /* length of line */
    /* pointer, end-pointer, rev-pointer and flag pointer-to-space */
    char *p = line + lnlen - 1, *ep = p, *rp = rline, *p2space = NULL;

    if (!line || !*line) {  /* validate line not NULL and not empty */
        fprintf (stderr, "revwords() error: 'line' empty of null.\n");
        return NULL;
    }

    if (*ep == '\n')    /* if line ends in '\n' -- remove it */
        *ep-- = 0;
    else                /* warn if no '\n' present in line */
        fprintf (stderr, "warning: no POSIX '\\n' found in line.\n");

    for (; ep >= line; ep--) {  /* for each char from end-to-beginning */
        if (*ep == ' ') {               /* is it a space? */
            size_t len = p - ep;        /* get the length of the word */
            strncat (rp, ep + 1, len);  /* concatenate word to rline  */
            if (p == line + lnlen - 1)  /* if first word, append ' '  */
                strcat (rp, " ");
            p = ep;                     /* update p to last ' '  */
            p2space = ep;               /* set flag to valid pointer */
        }
    }
    strncat (rp, line, p - line);       /* handle first/last word */

    if (!p2space) { /* validate line contained ' ', if not return NULL */
        fprintf (stderr, "revwords() error: nothing to reverse.\n");
        return NULL;
    }

    return rline;   /* return pointer to reversed line */
}

Note: if there is no '\n' present in line when passed to revwords, you are likely trying to read a line longer than SIZE chars (or you are reading the last line where there is no POSIX '\n' at the end of the file), and you need to handle that as required. Here I simply warn.

Example Use/Output

$ printf "my dog has fleas\nmy cat does too\n" | ./bin/str_rev_words

 line: my dog has fleas
rline: fleas has dog my

 line: my cat does too
rline: too does cat my

Look things over and let me know if you have any questions. There are dozens of ways to approach this problem, no one more right than another if they properly handle the reversal in a reasonable efficient manner. Take your pick.

If you like using the string library functions instead of pointer arithmetic, you could always do something like the following:

char *revwords (char *rline, char *line)
{
    /* length, pointer, end-pointer, pointer-to-space, copy of line */
    size_t len = strlen (line);
    char *p = NULL, *p2space = NULL, copy[len+1];

    if (!line || !*line) {  /* validate line not NULL and not empty */
        fprintf (stderr, "revwords() error: 'line' empty of null.\n");
        return NULL;
    }

    if (line[len-1] == '\n')    /* remove trailing newline */
        line[--len] = 0;
    else                /* warn if no '\n' present in line */
        fprintf (stderr, "warning: no POSIX '\\n' found in line.\n");

    strncpy (copy, line, len + 1);  /* copy line to 'copy' */

    /* for each ' ' from end-to-beginning */
    while ((p = strrchr (copy, ' '))) {
        strcat (rline, p + 1);          /* append word to rline */
        strcat (rline, " ");            /* followed by a space  */
        p2space = p;                    /* set p2space to p     */
        *p2space = 0;                   /* nul-terminate copy at p */
    }

    if (p2space) {              /* validate space found in line */
        *p2space = 0;           /* nul-terminate at space       */
        strcat (rline, copy);   /* concatenate first/last word  */
    }
    else {                      /* no ' ' in line, return NULL  */
        fprintf (stderr, "revwords() error: nothing to reverse.\n");
        return NULL;
    }

    return rline;   /* return pointer to reversed line */
}

Note: While not an error, the standard coding style for C avoids the use of caMelCase or MixedCase variable or funciton names in favor of all lower-case while reserving upper-case names for use with macros and constants. Leave caMelCase or MixedCase for java or C++. (it's style, so it's your choice, but it does say something about your code on first impression)

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
1

it seems you like difficulties.

Adopt this for your purposes

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

char* reverse_words(char* str);

int main() {
    char arr[] = "the big red dog"; 
    printf("%s", reverse_words(arr));
    return 0;
}


char* reverse_words(char* str) {
    char delim = ' '; // space  
    int left = 0;
    int reverse_index = 0;
    int right = 0;
    int len = strlen(str);
    char tmp;
    while (left < len) {
        while  (str[right] != delim && right < len)
            right++;    
        reverse_index = right - 1;
        while (left < reverse_index){
            tmp = str[left];
            str[left] = str[reverse_index];
            str[reverse_index] = tmp;
            left++;
            reverse_index--;
        }
        right++;        
        left = right;
    }

    strrev(str);
    return str;
}


//output is: dog red big the

if you have no strrev due to some reasons, here you are

char* strrev(char *str) {
  char *p1, *p2;

  if (! str || ! *str)
        return str;

  for (p1 = str, p2 = str + strlen(str) - 1;
                          p2 > p1; ++p1, --p2) {
        *p1 ^= *p2;
        *p2 ^= *p1;
        *p1 ^= *p2;
  }

  return str;

}

also more clear way, but more slow too

char* strrev(char *str) {
    int left = 0;
    int right = strlen(str) - 1;
    char tmp;
    while(left < right) {
        tmp = str[left];
        str[left] = str[right];
        str[right] = tmp;
        left++;
        right--;
    }

    return str;
}
bobra
  • 615
  • 3
  • 18
  • I hope you are aware that `strrev()` is not available on every platform. See [Is the strrev() function not available in Linux?](http://stackoverflow.com/questions/8534274/is-the-strrev-function-not-available-in-linux). – dtell Apr 10 '17 at 00:34
  • 1
    @datell added , hand made one, fo this case – bobra Apr 10 '17 at 00:41
  • Great! You could check the platform using the preprocessor like `#if defined(__MACH__) || defined(__linux__) the custom implementation #endif` or something similar. That would be very *C-styled* i guess – dtell Apr 10 '17 at 00:56
0
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

#define SIZE 80

char *reverseWords(char line[SIZE]){
    char temp[SIZE];
#if SIZE > 255
    unsigned index[SIZE];
#else
    unsigned char index[SIZE];
#endif
    int i, index_count = 0;
    int inside_word = !!isalpha((unsigned char)line[0]), word = inside_word;

    for(index[index_count++] = i = 0; line[i]; ++i){//copy & make index table
        unsigned char ch = temp[i] = line[i];
        if(inside_word && !isalpha(ch) || !inside_word && isalpha(ch)){//Edge
            index[index_count++] = i;
            inside_word = !inside_word;
        }
    }
    index[index_count] = i;

    int last_word_index = index_count - 1;//last index
    last_word_index -= !word  ^ (last_word_index & 1);//to word

    char *p =line;
    for(i = 0; i < index_count-1; ++i){
        int len;
        if(word){
            len = index[last_word_index+1] - index[last_word_index];
            memcpy(p, &temp[index[last_word_index]], len);
            last_word_index -= 2;
        } else {
            len = index[i+1] - index[i];
            memcpy(p, &temp[index[i]], len);
        }
        word = !word;
        p += len;
    }

    return line;
}

int main(void){
    char str[SIZE];

    while(fgets(str, sizeof str, stdin)){
        printf("%s", reverseWords(str));
    }
}
BLUEPIXY
  • 39,699
  • 7
  • 33
  • 70
0

Your problem would be simplified if you used more of the string.h functions, like strlen. Also, you must dynamically allocate memory with malloc and calloc--a fixed sized buffer will not do here.

I now present the revised reverseWords.

char *myrev(const char *line)
{
    char *revword(char *);

    size_t i = strlen(line);
    int inword = OUT;

    size_t nWord = 0, nallocWord;
    char *word;     // will store the word

    size_t nRet = 0, nallocRet;
    char *ret;      // will store the entire line, but reversed

    // establish preconditions
    assert(i > 0);
    assert(line != NULL);

    // alloc memory for word and ret
    if ((word = malloc(nallocWord = INITALLOC)) != NULL && 
                (ret = calloc(nallocRet = INITALLOC, sizeof(char))) != NULL) {

        // walk backwards through line
        while (i--) {
            if (inword == OUT && isalnum(line[i]))
                inword = IN;    // we just entered a word

            if (inword == IN && isalnum(line[i])) {
                // we're inside a word; append current char to the word buffer
                word[nWord++] = line[i];

                // word buffer exhausted; reallocate
                if (nWord == nallocWord)
                    if ((word = realloc(word, nallocWord += ALLOCSTEP)) == NULL)
                        return NULL;
            }

            // if we are in between words or at the end of the line
            if (i == 0 || inword == IN && isspace(line[i])) {
                inword = OUT;
                word[nWord] = '\0';

                word = revword(word);

                // ret buffer exhausted; reallocate
                if (nRet + nWord > nallocRet)
                    if ((ret = realloc(ret, nallocRet += ALLOCSTEP)) == NULL)
                        return NULL;

                // append word to ret
                strcat(ret, word);
                strcat(ret, " ");
                nRet += nWord + 1;

                nWord = 0;
            }
        }
        free(word);

        // remove trailing blank
        ret[strlen(ret) - 1] = '\0';
        return ret;
    }
    // in case of mem alloc failure
    return NULL;
}

I will now explain the operation of this function.

The first line declares the function revwords, which I will show later.

The next lines are the variable definitions. The variable i will be used as an iterator to walk backwards. We initialize it to the length of the line string, including the zero terminator.

The variable inword is important. It is used to keep track of whether we are inside a word or not. It will be assigned one of two constants: IN and OUT.

#define IN      0    /* inside a word */
#define OUT     1    /* outside a word */

The nWord and nallocWord variables are respectively the number of characters in the word buffer, and how much memory is allocated for word. word is where we will accumulate a word. Since the input line will be parsed backwards, the word buffer will be initially backwards, but we will later reverse it.

The variables nRet and nallocRet have similar purpose: they are respectively the number of characters in the ret buffer and the number of characters allocated for ret. ret is the buffer where we will store the entire input line, but with each word's position reversed.

We then enforce two preconditions: The length of the string must be positive, and the line input buffer must not be NULL. We enforce these by using the assert macro from <assert.h>.

We now enter the meat of the function. Our strategy in this function will be to seize a certain amount of memory initially for our word and ret buffers, and then later increase the size of our buffer if need be. So we do just that.

The line

if ((word = malloc(nallocWord = INITALLOC)) != NULL && 
                (ret = calloc(nallocRet = INITALLOC, sizeof(char))) != NULL) {

appears frightening at first, but if we split it into two parts, it will be easier. The part to the left of the AND operator allocates INITALLOC characters for word, and checks if the return value is not NULL (indicating failure). But INITALLOC is assigned to nallocWord, which, as we said earlier, is the number of characters allocated to word.

The part to the right of the AND allocates INITALLOC characters for ret, and checks if the return value is not NULL. But INITALLOC is assigned to nallocRet. Notice that we used the calloc function instead of malloc. The difference lies in the fact that calloc zero-initializes its return value, but malloc does not. We need our ret buffer to be zero-initialized; you will see why later.

#define INITALLOC 16   /* number of characters initially alloc'ed */
#define ALLOCSTEP 32   /* number of characters to increase by */

The values of these macros don't really matter, but you should still choose sensible values for them so that too many (slow) re-allocations aren't performed.

Anyway, inside of this if statement, we have the while loop which iterates the string line from the end. The while loop consists of a series of tests.

  1. If we are outside a word (inword == OUT) and the current character(line[i]) is alphanumeric(i.e, a character inside a word), then we change inword to IN. Control will fall through to the next if, which is

  2. If we are inside a word (inword == IN) and the current character is a word character, then we add the current character to the end of word, and increase the character count nWord. Within, we check if word has been exhausted, in which case memory is reallocated. If the reallocation fails, we return NULL. The reallocation works by increasing nallocWord by ALLOCSTEP, which is by how many characters we will resize our buffer.

  3. If we are in between words (inword == IN && isspace(line[i]), or if we are at the end of the line (i == 0), then we change inword to OUT, null-terminate word, and reverse it with a call to revword. Our next step is to add the word to the end of ret. However, we must first check if there is enough space for the concatenation to take place. The condition nRet + nWord > nallocRet checks if the number of characters in ret plus the number of characters in word exceeds nallocRet, which the number of characters allocated for the ret buffer. If the condition is true, memory is reallocated. If the reallocation fails, we return NULL. We need the check i == 0, because when the loop is about to finish, we want to push the final word into ret.

Now, we can append word to ret with a call to strcat. We also add a space, so that the words will have spaces between each other.

nRet is updated to the new number of characters in ret. The + 1 is to account for the space in between words. nWord is set to 0, so the next loop iteration will overwrite the old contents of word, which are no longer needed.

Once the loop is completed, we release word, since it is longer needed, and then remove the trailing space at the end of ret. We then return ret. It is the caller's responsibility, by the way, to release this memory. For every call to malloc/calloc, there must be a corresponding free.

Let us now turn to revword, which is the function to reverse a string.

char *revword(char *word)
{
    char *p, *q;

    assert(word != NULL);
    assert(*word != '\0');

    for (p = word, q = word + strlen(word) - 1; q > p; ++p, --q) {
        char tmp;

        tmp = *p;
        *p = *q;
        *q = tmp;
    }

    return word;
}

The function uses two character pointers, p and q. p is assigned to point to the start of word, whereas q is assigned to point to the end of word. The p pointer is incremented each loop iteration, and q is decremented, while q is greater than p. In the loop body, we swap the values pointed to by p and q.

Finally, we return the reversed word.

Now, I will show the little bit of main which I altered.

fgets(str, SIZE, stdin);
str[strlen(str) - 1] = '\0';

char *myrev(const char *line);

char *res = myrev(str);

printf("%s", res);
free(res);

This is inside the loop for (i = 0; i < N; i++).

We must remove the trailing newline from the str buffer, which fgets left in there. We then declare the myrev function, and next stored the return value of myrev in a temporary, so that we can use this pointer when we free it.

lost_in_the_source
  • 10,998
  • 9
  • 46
  • 75