Re: [PATCH v3 13/33] serial: start making SerialMM a sysbus device

[Marc-André Lureau]

Hi

On Wed, Nov 20, 2019 at 2:58 PM Peter Maydell <address@hidden> wrote:
>
> On Wed, 20 Nov 2019 at 10:40, Marc-André Lureau
> <address@hidden> wrote:
> > On Wed, Nov 20, 2019 at 2:36 PM Peter Maydell <address@hidden> 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" ?

In OOP, you implement methods for a specific type. "self" is of the
type that is being implemented.

serial_mm_foo(SerialMM *self);

serial_mm_instance_init(Object *o) {
  SerialMM *self = SERIAL_MM(o);
  ..
}


If you have a generic function, not tied to a specific class, this
rule doesn't apply.

do_this(some arg, SerialMM *smm)

>
> > > (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.
>

Fair enough

We can only change things incrementally, and while doing refactoring
is the right moment to write better and more readable code.


-- 
Marc-André Lureau