Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is this still being actively maintained? #930

Open
CoffeeTableEspresso opened this issue Feb 27, 2025 · 3 comments
Open

Is this still being actively maintained? #930

CoffeeTableEspresso opened this issue Feb 27, 2025 · 3 comments

Comments

@CoffeeTableEspresso
Copy link
Contributor

Looking at the repo, there are 183 (now 184) open issues, and 68 pull requests. Last commit to master was 5 months ago, last release was almost a year ago.

@FSMaxB and @Alanscut and @DaveGamble, is this repo being actively maintained?

If needed, I'm happy to help with some of the maintenance on this repo, I see some bugs/PRs that I think I could close out pretty easily, but if those PRs would not be accepted here, I'd rather just fork the repo and do it myself.

@intector
Copy link

Hey @CoffeeTableEspresso,
I'm not an insider and don't know any of the people you're addressing in your post, but I agree that nobody is working on this repo. I'm using this software in some of my projects and have found several errors or things that could be improved. However, I never posted anything here and instead modified my local fork to make it work for me.
I found several issues when I used this software in AZ-RTOS because some memory allocations are not consequent throughout the software. Even if one uses the memory allocation hooks to adapt to RTOS requirements, some functions ignore this and don't even bother allocating memory. One example is the 'print_value' function. If used on 'cJSON_False' or 'cJSON_True', it'll write in unallocated memory and crash with an 'Hard-Stop'.

Here is the 'print_value' function:

`/* Render a value to text. */
static cJSON_bool print_value(const cJSON * const item, printbuffer * const output_buffer)
{
unsigned char *output = NULL;

if ((item == NULL) || (output_buffer == NULL))
{
    return false;
}

switch ((item->type) & 0xFF)
{
    case cJSON_NULL:
        output = ensure(output_buffer, 5);
        if (output == NULL)
        {
            return false;
        }
        strcpy((char*)output, "null");
        return true;

    case cJSON_False:
        output = ensure(output_buffer, 6);
        if (output == NULL)
        {
            return false;
        }
        strcpy((char*)output, "false");
        return true;

    case cJSON_True:
        output = ensure(output_buffer, 5);
        if (output == NULL)
        {
            return false;
        }
        strcpy((char*)output, "true");
        return true;

    case cJSON_Number:
        return print_number(item, output_buffer);

    case cJSON_Raw:
    {
        size_t raw_length = 0;
        if (item->valuestring == NULL)
        {
            return false;
        }

        raw_length = strlen(item->valuestring) + sizeof("");
        output = ensure(output_buffer, raw_length);
        if (output == NULL)
        {
            return false;
        }
        memcpy(output, item->valuestring, raw_length);
        return true;
    }

    case cJSON_String:
        return print_string(item, output_buffer);

    case cJSON_Array:
        return print_array(item, output_buffer);

    case cJSON_Object:
        return print_object(item, output_buffer);

    default:
        return false;
}

}
`

In this case, I used a workaround, which looks like this:

if (dcu_def.s2lp_en) { cJSON_AddStringToObject(dcu_def_JSON, "DCU_SET_S2LP_EN", "true"); } else { cJSON_AddStringToObject(dcu_def_JSON, "DCU_SET_S2LP_EN", "false"); } // this is the original function used for Bool objects, but don't work here // cJSON_AddBoolToObject(dcu_def_JSON, "DCU_SET_S2LP_EN", dcu_def.s2lp_en);

@escherstair
Copy link

Hi @intector
I'm worried about your message.
Based on my impression, I suspect some corner cases inside cJSON library that don't handle very well memory allocation.
And so proper fixes are necessary.
If you can share your fork, this would be useful for other users too.

One question form my side:

  • your message states that print_value() doesn't work if called on either cJSON_False or cJSON_True, because it writes to unallocated memory
  • and so you use a workarounf that writes a string "true" or "false" instead of a boolean true or false
  • but why you say that print_value() writes to unallocated memory? It seems to me that it calls ensure() that should ensure (sic.) that the buffer has enough space

Did you find something different?

@CoffeeTableEspresso
Copy link
Contributor Author

@escherstair I agree with your impression, that this is a more fundamental problem with the library itself, rather than a small problem with booleans. And so proper fixes are needed, as you said, not a workaround. I would be happy to contribute to a fork to fix some problems.

@intector if you could share your fork, that would be helpful, so others can help/benefit from the fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants