On Fri Jun 3 09:56:05 2022 +0000, **** wrote:
=?UTF-8?Q?R=c3=a9mi_Bernon?= replied on the mailing list:
On 6/3/22 11:39, Dmitry Timoshkov wrote: > Rémi Bernon <wine@gitlab.winehq.org> wrote: > >> - char *buf, *cur, *tmp; >> + char *buf, *cur; >> int count = 0, buf_size = 16 * sizeof(struct hardware_msg_data); >> >> if (!req->buffer_size) buf = NULL; >> @@ -3373,13 +3373,13 @@ DECL_HANDLER(get_rawinput_buffer) >> if (cur + data->size > buf + buf_size) >> { >> buf_size += buf_size / 2 + extra_size; >> - if (!(tmp = realloc( buf, buf_size ))) >> + cur = (char *)(cur - buf); >> + if (!(buf = realloc( buf, buf_size ))) >> { >> set_error( STATUS_NO_MEMORY ); >> return; >> } >> - cur = tmp + (cur - buf); >> - buf = tmp; >> + cur = buf + (size_t)cur; >> } > > Reusing 'cur' as an offset doesn't look very elegant to me. Perhaps > a new variable to hold the offset could be more appropriate here? > I actually agree, I considered renaming the variable to "pos", which could maybe do better as both a position or an offset in the buffer, but maybe changing the code to use buf + offset everywhere would be better. It was a bigger diff though, so it made the change less obvious. -- Rémi Bernon <rbernon@codeweavers.com>
FWIW, I would also lean towards a dedicated variable with the offset, limited to inner scope for clarity. I don't think there's reasons to skimp on variables much these days..?