On Thu, 21 Jan 2021 09:44:51 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> -static void tsync_offset_load(struct tracecmd_input *handle, char *buf)
> +#define safe_read(R, C) \
> + { if ((C) > size) return -EFAULT; \
> + (R) = tep_read_number(tep, buf, (C)); \
> + buf += (C); \
> + size -= (C); }
> +static int tsync_offset_load(struct tep_handle *tep,
> + struct timesync_offsets *ts_offsets, char *buf, int size)
> {
> - struct host_trace_info *host = &handle->host;
> - long long *buf8 = (long long *)buf;
> + int start_size = size;
> int i, j;
>
> - for (i = 0; i < host->ts_samples_count; i++) {
> - host->ts_samples[i].time = tep_read_number(handle->pevent,
> - buf8 + i, 8);
> - host->ts_samples[i].offset = tep_read_number(handle->pevent,
> - buf8 + host->ts_samples_count + i, 8);
> - host->ts_samples[i].scaling = tep_read_number(handle->pevent,
> - buf8 + (2 * host->ts_samples_count) + i, 8);
> - }
> - qsort(host->ts_samples, host->ts_samples_count,
> + for (i = 0; i < ts_offsets->ts_samples_count; i++)
> + safe_read(ts_offsets->ts_samples[i].time, 8);
> + for (i = 0; i < ts_offsets->ts_samples_count; i++)
> + safe_read(ts_offsets->ts_samples[i].offset, 8);
> + for (i = 0; i < ts_offsets->ts_samples_count; i++)
> + safe_read(ts_offsets->ts_samples[i].scaling, 8);
I like to follow the Linux kernel requirements for macros. Which is any
complex macro like the "safe_read" above, needs to be done in a
do { } while (0) loop, as it is safer to use than just brackets. Also,
perhaps even making a macro out of the loop too.
#define safe_read(R, C) \
do { \
if (size < (C)) \
return -EFAULT; \
(R) = tep_read_number(tep, buf, (C)); \
buf += (C); \
size -= (C); \
} while (0)
#define safe_read_loop(type) \
do { \
int i; \
for (i = 0; i < ts_offsets->ts_samples_count; i++) \
safe_read(ts_offsets->ts_samples[i].type, 8); \
} while (0)
Then replace all the above with;
safe_read_loop(time);
safe_read_loop(offset);
safe_read_loop(scaling);
I'll look at this more tomorrow.
-- Steve
> + qsort(ts_offsets->ts_samples, ts_offsets->ts_samples_count,
> sizeof(struct ts_offset_sample), tsync_offset_cmp);
> /* Filter possible samples with equal time */
> - for (i = 0, j = 0; i < host->ts_samples_count; i++) {
> - if (i == 0 || host->ts_samples[i].time != host->ts_samples[i-1].time)
> - host->ts_samples[j++] = host->ts_samples[i];
> + for (i = 0, j = 0; i < ts_offsets->ts_samples_count; i++) {
> + if (i == 0 || ts_offsets->ts_samples[i].time != ts_offsets->ts_samples[i-1].time)
> + ts_offsets->ts_samples[j++] = ts_offsets->ts_samples[i];
> + }
> + ts_offsets->ts_samples_count = j;
> +
> + return start_size - size;
> +}
> +
|