Fix regression in text_handler_multiselect in the text frontend

In commit 299ecb85, we started to use readline to prompt the user, and as a result, the answer variable is now dynamically allocated (before, it used to be a 4096 chars array on the stack).

This broke two things.

First, the use of sizeof(answer) is not valid anymore, as it just returns the size of a pointer, not the size of the memory area.

Second, even if we fix the above to get the size right, we have another problem. There's no guarantee that answer is big enough to accomodate the final answer. If we take the example of the d-i tasksel screen, the answer from user is likely to be something like 1 7 12, but then it's converted to values, something like Debian desktop environment, ... MATE, standard system utilities, which is much longer. So if we want to reuse answer to hold the final answer, we'll need to reallocate it.

Therefore, with this commit, we go back to using a static 4096 chars buffer on the stack to hold the final answer, as it was before.

Closes: #xxxxxx

added 1 commit

• 32338730 - Fix regression in text_handler_multiselect in the text frontend

Compare with previous version

Apparently the idea of the original code was to just reuse the answer buffer, as it's supposed to have a correct size.

It seems that the code is currently broken just because the answer buffer happens not to be the right size. Apparently it has never been (since the user answer mostly answers just numbers, and doesn't put ,, and this loop is turning numbers into words, and adding ,). This doesn't seem related to my recent changes at all, but I guess some changes on the stack just happened to reveal the issue.

Here we just want to allocate buf to an appropriate size. You can easily compute an almost exact bound of that size: run a first loop that computes the sum of strlen(choices->choices[i]) + 2. Then allocate buf, and then you can use buf as you did (and remember to free it).

Thanks for your feedback. I updated the MR to use dynamic memory as you suggested.

Thanks for spotting it, please fix and we'll merge!

Ah, I hadn't seen the MR summary. Ok so it's the move to readline that broke it. That shows how fixed-size-buffers sloppiness leads to bugs :)

added 1 commit

• 8be0bfa1 - Fix regression in text_handler_multiselect in the text frontend

Compare with previous version

enabled an automatic merge when the pipeline for 8be0bfa1 succeeds

Marked for merging, thanks!

Oh wow, so fast! I didn't even test it yet ;) But I'll be testing it in the coming hours. If you don't hear from me again, it means that it's all good.

merged

mentioned in commit 0400a18a