On Mon, Jan 23, 2023 at 03:46:11PM +0200, Sakari Ailus wrote:
> Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera
> configuration. For now, only figure out where the descriptor is present in
> order to allow adding information from it to related devices.
...
> + memcpy(inst->remote_name, csi2->resource_source.string_ptr,
> + csi2->resource_source.string_length);
Why don't we use strscpy()? Is it really strings? Or is it some abuse of
the ACPI object type?
...
> +static acpi_status scan_check_crs_csi2(acpi_handle handle, u32 nesting_level,
> + void *context, void **ret)
> +{
> + struct scan_check_crs_csi2_context inst_context = {
> + .handle = handle,
> + .res_list = LIST_HEAD_INIT(inst_context.res_list),
> + };
> + struct list_head *list = context;
> + struct crs_csi2 *csi2;
> + INIT_LIST_HEAD(&inst_context.res_list);
Why do you need this? I don't see that variable is static...
> + acpi_walk_resources(handle, METHOD_NAME__CRS,
> + scan_check_crs_csi2_instance, &inst_context);
> +
> + if (list_empty(&inst_context.res_list))
> + return AE_OK;
> +
> + csi2 = kmalloc(sizeof(*csi2), GFP_KERNEL);
> + if (!csi2)
> + return AE_OK;
> +
> + csi2->handle = handle;
> + list_replace(&inst_context.res_list, &csi2->buses);
> + list_add(&csi2->list, list);
Hmm... Can list_swap() be used here?
> + return AE_OK;
> +}
...
> + /*
> + * Figure out how much temporary storage we need for counting
> + * connections in each device.
> + */
> + list_for_each_entry(csi2, &crs_csi2_handles, list) {
> + struct crs_csi2_instance *inst;
> +
> + handle_count++;
> + list_for_each_entry(inst, &csi2->buses, list)
> + handle_count++;
list_count_nodes()?
> + }
...
> + sort(handle_refs, handle_count, sizeof(*handle_refs), crs_handle_cmp,
> + NULL);
Yes, I would leave it on one line.
...
> + if (check_mul_overflow(sizeof(*ads->ports) +
> + sizeof(*ads->nodes) * 2 +
> + sizeof(*ads->nodeptrs) * 2,
> + (size_t)this_count, &alloc_size) ||
Can this_count be of size_t type from the beginning?
> + check_add_overflow(sizeof(*ads) + sizeof(*ads->nodes) +
> + sizeof(*ads->nodeptrs) * 2,
> + alloc_size, &alloc_size)) {
> + acpi_handle_warn(handle, "too many handles (%u)",
> + this_count);
> + continue;
> + }
...
> + ads->nodeptrs = (void *)(ads->nodes +
> + this_count * 2 + 1);
Why this is not on one line? (I have got less than 80).
--
With Best Regards,
Andy Shevchenko
