1

When I am running this variadic function I am getting warning array subscript has type 'char', and then it crashes. I suspect the problem occurs somewhere in the while loop. I already tried many methods to prevent this function from crashing. For example: I tried changing (argp, char) to (argp, int) but this will not produce correct output . I also not allowed to change the prototype function. Please help!

char fancyMostFrequentChar(char c, ...) {
    char j;
    int max = 0;
    int fre[255] = { 0 };
    char temp;
    va_list argp;

    if (c == '\0')
        return '\0';
    va_start(argp, c);

    //This for loop will run through the arguments
    //and record their frequencies into fre[] array
    while ((temp = va_arg(argp, char)) != '\0') {
        ++fre[temp];
        if (fre[temp] > max) {
            max = fre[temp];
            j = temp;
        }
    }
    va_end(argp);

    return j;
}
4
  • 5
    Type char can (implementation defined) be signed, so the index can be negative. Commented Sep 3, 2017 at 19:47
  • The first element is not processed. In case of only the first element(E.g 'c','\0'), an uninitialized variable j is returned. and va_arg(argp, char) --> va_arg(argp, int) Commented Sep 3, 2017 at 19:52
  • 2
    Aside: int fre[255] ==> int fre[256] Commented Sep 3, 2017 at 19:56
  • The warning is almost certainly emitted by your compiler. It appears right before the crash only because you run the program immediately after compiling it (possibly because your IDE performs a compiler + run combination via a single control). But that does not mean the warning is unrelated to the crash. Commented Sep 3, 2017 at 20:06

3 Answers 3

2

You should definitely make temp an unsigned char or use a cast as you use it to index into the array fre. char can be, and often is, signed by default so any negative values would access outside the boundaries of the array. By the way, you should make fre one item larger to accommodate for all 8-bit values, including 255.

Note that you do not count the first argument, so fancyMostFrequentChar('a', 'b', 'a', '\0') would return 'b' instead of 'a'.

Note also (as commented by M.M) that the first argument should be defined as int for va_start to have defined behavior:

7.16.1.4 The va_start macro

Synopsis

#include <stdarg.h>
void va_start(va_list ap, parmN);

Description

The va_start macro shall be invoked before any access to the unnamed arguments.

The va_start macro initializes ap for subsequent use by the va_arg and va_end macros. [...]

The parameter parmN is the identifier of the rightmost parameter in the variable parameter list in the function definition (the one just before the , ...). If the parameter parmN is declared with the register storage class, with a function or array type, or with a type that is not compatible with the type that results after application of the default argument promotions, the behavior is undefined.

You mention you are not supposed to change the prototype... Unfortunately the prototype as posted char fancyMostFrequentChar(char c, ...) cannot be used to access the arguments past the first one.

Here is a corrected version:

char fancyMostFrequentChar(int c, ...) {
    int fre[256] = { 0 };
    char temp, res = (char)c;

    if (res != '\0') {
        va_list argp;
        va_start(argp, c);

        ++fre[(unsigned char)res];    
        //This for loop will run through the arguments
        //and record their frequencies into fre[] array
        while ((temp = va_arg(argp, char)) != '\0') {
            if (++fre[(unsigned char)temp] > fre[(unsigned char)res]) {
            res = temp;
        }
        va_end(argp);
    }
    return res;
}
Sign up to request clarification or add additional context in comments.

Comments

1

va_arg(argp, char) is undefined behaviour. The use of va_arg macro requires that the type be unchanged under the default argument promotions -- which means you can't use float, nor any integer type narrower than int. (Character types are integer types). Reference: C11 7.16.1.1/2

It is also undefined behaviour to have the second argument to va_start be of a type that changes under the default argument promotions. So it is actually impossible to correctly access the variadic arguments for a function declared as char fancyMostFrequentChar(char c, ...).

To fix this you must change the prototype of the function, for example:

char fancyMostFrequestChar(int c, ...)

and the relevant part of your loop should be:

int temp;

while ((temp = va_arg(argp, int)) != '\0')
{
    unsigned char index = (unsigned char)temp;

and then use index as your array index. It is then guaranteed to be in the range of 1 through to UCHAR_MAX (which is usually 255). For pedantic correctness you could change your array definition to be int fre[UCHAR_MAX+1].

Comments

0

Your code has a lot of problems, I hope I will catch all.

unsigned char fancyMostFrequentChar(unsigned char c, ...) // resolve ambigity of char
{
    unsigned char j = '\0';   // was not initialized 
    int max = 0;
    int fre[256] = {0};    // max unsigned char is 255 -> array length = 256 (0..255)
    unsigned char temp;
    va_list argp;

    if (c == '\0')
        return '\0';
    va_start(argp, c);

    //This for loop will run through the arguments
    //and record their frequencies into fre[] array
    while ((temp = va_arg(argp, unsigned char)) != '\0')
    {
        ++fre[temp];
        if (fre[temp] > max)
        {
            max = fre[temp];
            j = temp;
        }
    }

    va_end(argp);

    return j;
}

3 Comments

The memset is not required, int fre[256] = {0}; initializes also all values to 0.
Hi Stefan, thank you for your answer. Unfortunately I am not allowed to change the function i.e from char to unsigned char because that is how the program will be graded.
@Lucas Jack: The day will come ;-)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.