On Wed, 20 Nov 2019 at 10:40, Marc-André Lureau <marcandre.lur...@gmail.com> wrote: > On Wed, Nov 20, 2019 at 2:36 PM Peter Maydell <peter.mayd...@linaro.org> > wrote: > > There are two problems here: > > (1) "self" gives no hint at all about whether it's the Object*, > > the DeviceState*, or the SerialMM*. All of those are the > > object the method is operating on, but the type differences matter. > > "self" should have the type of the object that is being implemented. > > serial_mm_instance_init -> SerialMM *self
In a typical device implementation, some functions take the object as a "SerialMM" or other specific pointer. Some take an Object*. Some take a DeviceState*. Some take void*. Which of those is "the type of the object that is being implemented" ? > > (2) *Absolutely nobody anywhere else in any other device model > > is using the 'self' argument name*. It's not a convention if > > only this one file is using it, and adopting it here gives us > > absolutely no consistency -- exactly the opposite. > > Since there is no clear convention, then adding "self" isn't breaking > any convention. There is a clear convention, which I explained to you already (variable is named with some abbreviation/initialism of the type name, or sometimes just 's' for "state"). You are trying to write code which ignores that convention and does something else *which no other code is doing*. Please stop doing that -- in particular, don't do it in one device in the middle of a refactoring series. If you want to argue that we should change our convention for how we write QOM code, feel free -- but that should be a separate discussion and probably ought to come with a plan for how to do a general update of the codebase to avoid a weird mix of styles. thanks -- PMM
