On 10/15/2024 1:04 PM, Bjorn Andersson wrote:
On Tue, Oct 15, 2024 at 12:34:29PM -0600, Jeffrey Hugo wrote:
On 10/14/2024 3:52 PM, Bjorn Andersson wrote:
On Wed, Sep 18, 2024 at 09:52:54AM -0600, Jeffrey Hugo wrote:
+ dev_table_entry = (struct sahara_debug_table_entry64 *)(context->rx);
+ for (i = 0; i < table_nents; ++i, ++image_out_table_entry, ++dev_table_entry) {
+ image_out_table_entry->type = le64_to_cpu(dev_table_entry->type);
+ image_out_table_entry->address = le64_to_cpu(dev_table_entry->address);
+ image_out_table_entry->length = le64_to_cpu(dev_table_entry->length);
+ strscpy(image_out_table_entry->description, dev_table_entry->description,
+ SAHARA_TABLE_ENTRY_STR_LEN);
+ strscpy(image_out_table_entry->filename,
+ dev_table_entry->filename,
+ SAHARA_TABLE_ENTRY_STR_LEN);
+ }
+
+ context->mem_dump_freespace = image_out_table_entry;
+
+ /* Done parsing the table, switch to image dump mode */
+ context->dump_table_length = 0;
+
+ /* Request the first chunk of the first image */
+ context->dump_image = (struct sahara_dump_table_entry *)(context->mem_dump +
+ sizeof(*dump_meta));
I would have preferred to see this (and above) written such that it's
explicitly clear that you're filling out an array of entries and then
point this to the first entry in that array.
I'm not sure I understand what you would like to see here. Can you perhaps
give an example?
Per your devcoredump definition at the top, image_out_table_entry is an
array of struct sahara_dump_table_entry, which you fill out by sliding a
pointer starting at mem_dump + sizeof(*dump_meta).
You then have context->dump_image to be a pointer to each element in
this array, except that it's not expressed as an array...
But it took me a minute to understand that this was what the code is
doing.
If you instead wrote it as:
for (i = 0..table_nents) {
image_out_table[i].foo = bar;
...;
}
context->dump_image = &image_out_table[0];
(Or perhaps even make dump_image an index into image_out_table)
It would have been obvious to me when I looked at the code that you're
setting up an array and then looping over each entry in the array.
So, I don't see anything wrong with the logic, but it would have been
easier for me if the code manifested this array, as an array...
Perhaps I'm missing some detail which complicates things, as far as I
can tell the logic presented is reasonable.
Ok, I see what you mean, and I believe its possible to transform the
logic to use array notation in the loop as you suggest.
-Jeff
