On Sat, May 16, 2020 at 12:13:37PM -0700, [email protected] wrote:
> (Not sure if this belongs in tech@ or misc@)

I think tech@ is fine for discussion of code.

> 
> What is the thinking around non-functional code changes that just
> improve clarity without functionality changes?  I can imagine bad
> experiences with that, but there is certainly room for improvement.

Split out from any functional changes I don't see a problem.

> 
> For instance, in the wsdisplay_emulops structure, there are:
> 
> int   (*alloc_attr)(void *c, int fg, int bg, int flags, long *attrp);
> void  (*unpack_attr)(void *c, long attr, int *fg, int *bg, int *ul);
> 
> And at the end of the structure is this comment, showing that at
> least someone (other than me) was confused by it:
>       /* XXX need a free_attr() ??? */
> 
> I would suggest that alloc_attr should be renamed to pack_attr, but
> there are 84 matches on alloc_attr across 39 unique files.

While alloc_attr() predates unpack_attr() I agree that pack_attr()
would be a better name.  And yes it seems quite a few drivers
across multiple architectures would need to be changed.

> 
> Similarly, I am taking notes as I read the code, and adding a few
> comments in various files and functions could help other people get
> up to speed much faster in the future, but I am hesitant to submit
> patches. I am hoping to write a decent article about my explorations,
> but inline comments are more durable documentation.

Comments here sound like a good thing.  wscons originated in NetBSD and
many of the people who have hacked on it in OpenBSD over the years are
no longer around.

Reply via email to