[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 03/16] x86/monitor: mechanical renames
On Sat, Jul 9, 2016 at 12:46 PM, Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx> wrote: > On 7/9/2016 9:10 PM, Tamas K Lengyel wrote: >> >> On Fri, Jul 8, 2016 at 10:13 PM, Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx> >> wrote: >>> >>> Arch-specific vm-event functions in x86/vm_event.h -e.g. >>> vm_event_init_domain()- >>> don't have an 'arch_' prefix. Apply the same rule for monitor functions - >>> originally the only two monitor functions that had an 'arch_' prefix were >>> arch_monitor_domctl_event() and arch_monitor_domctl_op(), but I gave them >>> that >>> prefix because -they had a counterpart function in common code-, that >>> being >>> monitor_domctl(). >> >> This should actually be the other way around - ie adding the arch_ >> prefix to vm_event functions that lack it. > > > Given that the majority of the arch-specific functions called from > common-code don't have an 'arch_' prefix unless they have a common > counterpart, I was guessing that was the rule. It made sense in my head > since I saw in that the intention of avoiding naming conflicts (i.e you > can't have monitor_domctl() both on the common-side and on the arch-side, so > prepend 'arch_' to the latter). I noticed you guys also 'skip' the prefix > when sending patches, so that reinforced my assumption. > >> Having the arch_ prefix is >> helpful to know that the function is dealing with the arch specific >> structs and not common. > > > Personally I don't see much use in 'knowing that the function is dealing > with the arch structs' from the call-site and you can tell that from the > implementation-site just by looking at the path of its source file. Also, > the code is pretty much localized in the arch directory anyway so usually > one wouldn't have to go back and forth between common and arch that often. > What really bothers me though is always having to read 'arch_' when spelling > a function-name and also that it makes the function name longer without much > benefit. Your suggestion of adding it to pretty much all functions that make > up the interface to common just adds to that headache. :-D > >> Similarly that's why we have the hvm_ prefix >> for functions in hvm/monitor. > > > 'hvm_' doesn't seem to me more special than 'monitor_', for instance, but > maybe that's just me. > >>> Let this also be the rule for future 'arch_' functions additions, and >>> with this >>> patch remove the 'arch_' prefix from the monitor functions that don't >>> have a >>> counterpart in common-code (all but those 2 aforementioned). >> >> Even if there are no common counter-parts to the function, the arch_ >> prefix should remain, so I won't be able to ack this patch. >> >> Tamas > > > Having said the above, are you still of the same opinion? Yes, I am. While it's not a hard rule to always apply these prefix, it does make sense to have them so I don't see benefit in removing the existing prefixes. Adding arch_ prefix to the ones that don't already have one is optional, I was just pointing out that if you really feel like standardizing the naming convention, that's where I would like things to move towards to. Tamas _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |