On Thu, 28 Apr 2011, Colin McCabe wrote:
> On Thu, Apr 28, 2011 at 11:27 AM, Yehuda Sadeh Weinraub
> <[email protected]> wrote:
> > On Thu, Apr 28, 2011 at 11:24 AM, Yehuda Sadeh Weinraub
> > <[email protected]> wrote:
> >>>
> >>> What do you guys think of changing struct ceph_mount_t to struct
> >>> ceph_mount_info_t?
> >>>
> >> I personally don't like sticking a _t prefix to a struct. It's either
> >> a typedef or a struct.
> >>
> > Ahrm.. *postfix*
> >
> 
> I'm ok with removing the _t stuff from our library APIs. But it we'd
> have to bump up the revision numbers on everything :P
> 
> I'm just going to leave this here:
> 
> http://ceph.newdream.net/git/?p=ceph-client.git;a=blob;f=Documentation/CodingStyle;h=8bb37237ebd25b19759cc47874c63155406ea28f;hb=HEAD

Ok, I'll bite.  ceph_mount_info_t -> struct ceph_mount_info *, etc.

If we do the same on librados it'd be a rev change, yep.  It would also 
improve compiler warnings.. I was bit last night by

  rados_aio_create_completion(0, 0, 0, &a);
  rados_aio_write(io_ctx, "a", a, buf, 100, 0);
vs
  rados_aio_write(io_ctx, "a", &a, buf, 100, 0);

because the typedefs are all void*.

sage

> 
> >             Chapter 5: Typedefs
> >
> > Please don't use things like "vps_t".
> >
> > It's a _mistake_ to use typedef for structures and pointers. When you see a
> >
> >     vps_t a;
> >
> > in the source, what does it mean?
> >
> > In contrast, if it says
> >
> >     struct virtual_container *a;
> >
> > you can actually tell what "a" is.
> >
> > Lots of people think that typedefs "help readability". Not so. They are
> > useful only for:
> > [...]
> >
> 
> I'll come back once the flamewar has settled down :)
> 
> Colin
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to