1

I've been tasked to write a recursive function in C that changes a string of any chars to all caps (without using toupper() or any other function for that matter).

This is the previous code and attempt to solve the question:

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

char* allCapStringRec(char str[])
{
    if (str[0] == '\0' )
        return 0; 

    if (str[0] >= 'a' && str[0] <= 'z')
        str[0] = str[0] - 32;

    return allCapStringRec(str+1);
}

The code needs to compile successfully through a "test" to check the code - The function works by itself but doesn't complete that testing.

The testing:

void Test3(char str[], char expected[], int dec)
{
    char *result = allCapStringRec(str);

    if (strcmp(result, expected) != 0)
        printf("allCapStringRec => Your Output is %s, Expected: %s\n", result, expected);
}

int main()
{
    Test3("123321", "123321", 4);
    Test3("abBba", "ABBBA", 4);
    Test3("ab$cd", "AB$CD", 4);

    printf("done");
    return 0;
}

my output:

dupCapStringRec => Your Output is , Expected: 123321

Sorry for all the edits, it's my first question here. I need help knowing what I'm doing wrong:)

  • 2
    Edit the question to provide a [mre]. – Eric Postpischil Jun 16 '22 at 14:18
  • 2
    [what's the problem?](https://godbolt.org/z/Mrd8h99d3) – yano Jun 16 '22 at 14:20
  • Questions seeking debugging help should generally provide a [mre] of the problem, which includes a function `main` and all `#include` directives. This allows other people to easily test your program, by simply using copy&paste. – Andreas Wenzel Jun 16 '22 at 14:20
  • 4
    The problem is certainly in the code that calls `allCapStringRec`. You probably passing a string literal to the function. [Edit] and show a [mcve], then we can tell you more. – Jabberwocky Jun 16 '22 at 14:24
  • 1
    "but it doesn't seem to work." -- [Blanket statements such as "it doesn't work" are not helpful.](https://idownvotedbecau.se/itsnotworking/) Please specify exactly the desired output and the actual output. – Andreas Wenzel Jun 16 '22 at 14:24
  • Does the calling code treat the return value as a pointer to the modified string? Does it check for `NULL`. What is the return value supposed to be? – Gerhardh Jun 16 '22 at 14:33
  • 2
    Given the edit, @Jabberwocky was spot on, the proper duplicate should be [Why do I get a segmentation fault when writing to a "char *s" initialized with a string literal, but not "char s\[\]"?](https://stackoverflow.com/q/164194/2505965) – Oka Jun 16 '22 at 14:41
  • 3
    Your code has both problems that have already been mentioned. The function always returns 0. You try to modify a string literal. Are you allowed to make changes to `main` and/or `Test3`? If not, you'll need to allocate memory for the result string, and copy the input string into that memory. – user3386109 Jun 16 '22 at 14:42
  • 1
    It's not a major problem, but the `dec` argument to `Test3()` is unused. It should be removed. – Jonathan Leffler Jun 16 '22 at 14:52
  • Side note: don't use magic numbers. Instead of `32` use `'a' - 'A'` which clearly shows your intention. – Jabberwocky Jun 16 '22 at 14:57
  • 1
    Strictly speaking, `if (str[0] >= 'a' && str[0] <= 'z')` isn't portable, as standard C makes no guarantee of these being adjacent. Same with `str[0] = str[0] - 32;`. Some pedantic language-lawyer might come and whine about portability to [EBCDIC](https://en.wikipedia.org/wiki/EBCDIC). – Lundin Jun 16 '22 at 14:58
  • 1
    It would be easier for other people to test your program if you added all necessary `#include` directives. Otherwise, somebody who wants to test your program will have additional work finding out which `#include` directives are necessary. – Andreas Wenzel Jun 16 '22 at 14:59
  • The last comments with more than 1 upvote explain the actual problem. But anyway: you should [edit] and show the actual output you get. – Jabberwocky Jun 16 '22 at 14:59
  • `if (strcmp(result, expected) != 0)` This confirms what I mentioned in my earlier comment. You don't check `result` for `NULL` and as it will always be `NULL` you cause undefined behaviour even if you solve the problem with string literals. – Gerhardh Jun 16 '22 at 15:01

3 Answers3

1

This is a terrible use of recursion! That's out of the way, so how can we fix this code:

The other answers, so far, assume you can change Test3(), I'm assuming you can't -- it's a test routine being applied to your code by someone else. If so, I see two ways to deal with it's call:

char *result = allCapStringRec(str);

First, is to assume we can compile this code with writable strings --that is we're allowed to modify any string that gets passed in. In which case, having allCapStringRec() return a value is just a convenience and not an integral feature of it's recursion:

char *allCapStringRec(char str[])
{
    // assumes compiled with -fwritable-strings

    if (str[0] == '\0') {
        return str;
    }

    if (str[0] >= 'a' && str[0] <= 'z') {
        str[0] -= ' ';
    }

    allCapStringRec(str + 1);

    return str;  // convenience return, not strictly necessary
}

If we can't assume writable strings, things get more complicated as we have to create a new string and return that. Here the returning of a string result becomes integral to the recursion:

char *allCapStringRec(char str[])
{
    char *upper = calloc(strlen(str) + 1, sizeof(char));  // calloc initializes to '\0'

    if (str[0] == '\0') {
        return upper;
    } else if (str[0] >= 'a' && str[0] <= 'z') {
        upper[0] = str[0] - ' ';
    } else {
        upper[0] = str[0];
    }

    char *tail = allCapStringRec(str + 1);

    (void) strcpy(upper + 1, tail);

    free(tail);

    return upper;
}

If this is what was assumed, then Test3() is flawed and should free(result) as the last thing it does to avoid a memory leak. But using calloc() and free() violates your "without using toupper() or any other function" requirement so this can't be solved for non-writable strings.

cdlane
  • 40,441
  • 5
  • 32
  • 81
0

Your code is wrong, but however it's not a total failure.

This needs to be changed:

  • Don't make the allCapStringRec return a pointless pointer but make it a void function.
  • Don't try to modify string literals (they can't be modified), but copy the string literals to a temporary buffer and do the transformation on that buffer. More information about this problem about string literals.

More explanations in the comments:

void allCapStringRec(char str[])  // make the function void
{
  if (str[0] == '\0')
    return;                       // don't return anything

  if (str[0] >= 'a' && str[0] <= 'z')
    str[0] = str[0] - 32;

  allCapStringRec(str + 1);       // don't return anything
}


void Test3(char str[], char expected[], int dec)
{
  char testbuffer[100];           // temporary buffer that can be modified
  strcpy(testbuffer, str);        // copy source to temp buffer
  allCapStringRec(testbuffer);    // convert string in temp buffer

  if (strcmp(testbuffer, expected) != 0)   // compare converted testbuffer with expected
    printf("allCapStringRec => Your Output is %s, Expected: %s\n", testbuffer, expected);
}
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
0

Problem 1: You attempt to modify read-only string literals. Why do I get a segmentation fault when writing to a "char *s" initialized with a string literal, but not "char s[]"? Instead, you must pass a writable string to the function since you intend to change the string in-place.

Problem 2: if (str[0] >= 'a' && str[0] <= 'z') str[0] = str[0] - 32; isn't really portable code.

Problem 3: recursion for the sake of recursion (and now we have 3 problems).


You can somewhat more efficiently and a lot more portably implement this function as:

(This can also be quite easily modified to cover locale: French, Spanish, German etc)

void str_toupper (char* str)
{
  static const char LUT[127] =
  {
    ['a'] = 'A',
    ['b'] = 'B',
    ['c'] = 'C',
    ['d'] = 'D',
    ['e'] = 'E',
    /* you get the idea */
  };

  for(size_t i=0; str[i] != '\0'; i++)
  {
    char converted = LUT[ str[i] ];
    if(converted)
      str[i] = converted;
  }
}

Usage:

char str[] = "Hello World!";
str_toupper(str);

The trick here is that the look-up table sets all items not used to zero and those are ignored during upper-case conversion.


If you want to use recursion just for the heck of it, then this would be that:

#ifdef MUST_USE_RECURSION_FOR_REASONS_UNKNOWN

void str_toupper (char* str)
{
  static const char LUT[127] =
  {
    ['a'] = 'A',
    ['b'] = 'B',
    ['c'] = 'C',
    ['d'] = 'D',
    ['e'] = 'E',
    /* you get the idea */
  };

  if(*str == '\0')
    return;

  char converted = LUT[*str];
  if(converted)
    *str = converted;  
  str_toupper(str+1); // tail call
}

#endif
Lundin
  • 195,001
  • 40
  • 254
  • 396