On 4/16/25 2:50 PM, Michael S. Tsirkin wrote:
On Wed, Apr 16, 2025 at 09:29:23AM -0400, Alejandro Jimenez wrote:
On 4/16/25 7:36 AM, Sairaj Kodilkar wrote:
+static int amdvi_as_to_dte(AMDVIAddressSpace *as, uint64_t *dte);
+
No need to have this function declaration as it is a static function.
I am adding a forward declaration since this function will be used by
several other new and existing functions throughout the series, and this
avoids having to keep moving the function definition. I do the same for many
other functions in later patches, since it is more practical than constantly
moving definitions around to guarantee ordering constraints. That approach
would create patches with large diffs but non-functional changes due to code
movement alone, makes it harder to parse the changes that actually matter,
harder to rebase and resolve conflicts, etc...
Alejandro
You can add forward declarations temporarily to simplify review. But in
the end, I prefer seeing code without forward declarations, with
functions ordered sensibly by order of calls, rather than spread
randomly all over the place.
I see. I did try to add the new functions in locations where they would
logically fit, instead of randomly spread. But as the patchset grew it
became harder to do without code movement.
I'll rebase the series to avoid using forward declarations.
In a somewhat related note, there are several patches that introduce
helpers that are not immediately used, so I marked the definitions with
the unused attribute, to be removed later. I can obviously squash the
helper patches with the later ones adding the functionality, but I chose
the more modular approach to split the patches. Please let me know if
you agree with this approach, or you'd like me to submit larger patches
instead.
Thank you,
Alejandro
Regards
Sairaj Kodilkar
