Overflow issue with Roller/Drop Down List set options

Issue with Roller/Drop Down List set options.

CPU: Xilinx Zynq

What do you experience?

I am creating a large list of ‘wordy’ objects using a roller based on a file downloaded from the Internet. This code has been working happily for many months now, but today the system hung. Upon investigation it seems I have an issue with the function:

void lv_ddlist_set_options(lv_obj_t * ddlist, const char * options);

The variable “i” which is declared on line 188 of lv_ddlist.c is a uint16_t, this limits the maximum text buffer size passed to the function for processing to 64 Kilobytes. In my case I passed about 66 Kilobytes which caused the variable “i” to wrap so the function never returns as it can not advance far enough into the text buffer to find the terminating NULL character.

What do you expect?

Is there any reason why the variable “i” can not be declared as a uint32_t removing this limitation?

Kind Regards,

Pete

It seems that you always find the bugs relating to large amounts of text. :slightly_smiling_face:

The root problem here is that there are many internal APIs which use uint16_t to reference a character’s index in a string. Changing uint16_t to uint32_t in lv_ddlist alone will not end well, from what I see.

The cleanest solution I can think of that avoids an excessive memory consumption increase for people is to add a new typedef to lv_conf.h that allows you to choose whether text is indexed using 16-bit or 32-bit variables.

@kisvegabor What do you think about this solution?

1 Like

@embeddedt yes sorry about that, it’s just that one of the projects I am currently working on does indeed push the boundaries in those areas… :blush:

To keep my project going for now I have just changed my local copy of littlevgl to use a uint32_t at line 188 of the source file, which appears fine for the time being, while I continue with other things…

If @kisvegabor agrees with your suggestion, that will be absolutely fine with me. :grinning:

Thank you.

Pete

1 Like

My suggestion can only safely be applied to dev-7.0 (as it changes the ABI and API) so you’ll have to stick with your patch or backport mine.

1 Like

Hi @embeddedt,

Thank you for the update, if you can let me know when you get around to applying the fix to dev-7.0 I would much appreciate it. I will then look at updating my projects to version 7.0 at that point.

Kind Regards,

Pete

Do you think the increase in memory usage would be notable? Right now I don’t remember too many places where string indexes are stored directly. They are rather only local variables and function parameters. However, the number of options, rows or similar things are usually uint16_t and they are stored in objects’ ext data. Do you think this limit can be reached as well?

I can’t imagine any user interface where someone would want to scroll through 65,536 items to find what they were looking for.

The memory usage probably won’t be too much of a problem; I was thinking more about performance. On 16-bit platforms, I don’t know how much more expensive passing a 32-bit variable between functions is when compared to 16-bit variables.

Fine, I agree.

As far as I know, it simply takes 2 registers to pass 32-bit variables. However, I’m sure it won’t cause any notable difference.

I’ll apply this fix within the next few days.

1 Like

Is there any downside if using size_t instead of uint32_t? AFAIK They are the same size (in bytes) on 32bit CPUs, like ARM Cortex M, most functions related to sizes, or lenghts, uses size_t.

uint32_t seems to be used in a number of spots elsewhere in the text code, so I’d rather stick with that for consistency. On a 64-bit CPU, size_t can be 8 bytes, and that wouldn’t match the uint32_t values already in use.