Re: [Linuxptp-devel] [PATCH RFC 00/30] Convert open coded interface into proper module.

2020-03-04 Thread Richard Cochran
On Tue, Feb 18, 2020 at 10:03:38AM -0800, Jacob Keller wrote:
> 
> Nice. A lot of patches, but they sound reasonable. Will review them this
> afternoon.

I really appreciate your careful review.  It is quite a chore.

Thanks,
Richard



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH RFC 00/30] Convert open coded interface into proper module.

2020-02-18 Thread Jacob Keller
On 2/11/2020 6:03 AM, Richard Cochran wrote:
> Using gcc8 and -O2, the compiler emits an annoying false positive
> warning.  Since I want to have -Werror, I went about fixing it.  Start
> pulling on a thread, and ...
> 

Excellent goal!

> The 'struct interface' is wide open to its users, and each user
> fiddles with the fields in a different way.  The cure is, of course,
> to hide the implementation details in a source module.  This task
> turned out to be harder than it looked at first.
> 

Yea this can be quite challenging...

> - Patch #1 refactors the makefile a bit (this will be useful in a series to 
> follow)
> - Patches 2-3 clean up interfaces accepting character strings as parameters.
> - Patch #5 introduces the "interface" module.
> - Patches 6-23 convert all the users to proper methods, one method at a time.
> - Patches 24-27 add create/destroy methods.
> - Patch #28 hides the implementation of the struct.
> - Patch #29 _finally_ fixes the compiler warning.
> - Patch #30 removes left over crud.
> 

Nice. A lot of patches, but they sound reasonable. Will review them this
afternoon.

Regards,
Jake

> Thanks for your review,
> 
> Richard Cochran (30):
>   Group related objects together within the makefile.
>   config: Constify the public interface.
>   rtnl: Constify the public interface.
>   utils: Constify the posix clock interface.
>   Move the network interface into its own header file.
>   interface: Introduce an access method for the name field.
>   Convert call sites to the proper method for getting interface names.
>   interface: Introduce an access method for the time stamping label.
>   Convert call sites to the proper method for getting interface labels.
>   interface: Introduce a method to get the time stamping information.
>   Convert call sites to the proper method for getting time stamp
> information.
>   interface: Introduce a method to initialize the time stamping label.
>   Convert call sites to the proper method for initializing the time
> stamping label.
>   interface: Introduce a method to set the name.
>   Convert call sites to the proper method for setting the name.
>   interface: Introduce a method to set the time stamping label.
>   Convert call sites to the proper method for setting the time stamping
> label.
>   interface: Introduce a method to get the PHC index.
>   Convert call sites to the proper method for getting the PHC index.
>   interface: Introduce a method to test the time stamping information
> validity.
>   Convert call sites to the proper method for testing time stamp info
> validity.
>   interface: Introduce a method to test supported time stamping modes.
>   Convert call sites to the proper method for testing time stamping
> modes.
>   interface: Introduce methods to create and destroy instances.
>   clock: Use the proper create/destroy API for network interfaces.
>   config: Use the proper create/destroy API for network interfaces.
>   pmc: Use the proper create/destroy API for network interfaces.
>   interface: Hide the implementation details.
>   interface: Silence warning from gcc version 8.
>   interface: Remove obsolete method.
> 
>  clock.c| 61 
>  config.c   | 26 --
>  config.h   | 19 ++
>  interface.c| 78 +
>  interface.h| 94 ++
>  makefile   | 32 +
>  nsm.c  | 18 ++
>  pmc_common.c   | 19 +-
>  port.c | 59 +--
>  port_private.h |  2 +-
>  raw.c  |  5 +--
>  rtnl.c |  6 ++--
>  rtnl.h |  8 +++--
>  udp.c  |  4 +--
>  udp6.c |  4 +--
>  uds.c  |  8 ++---
>  util.c |  2 +-
>  util.h |  2 +-
>  18 files changed, 316 insertions(+), 131 deletions(-)
>  create mode 100644 interface.c
>  create mode 100644 interface.h
> 


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCH RFC 00/30] Convert open coded interface into proper module.

2020-02-11 Thread Richard Cochran
Using gcc8 and -O2, the compiler emits an annoying false positive
warning.  Since I want to have -Werror, I went about fixing it.  Start
pulling on a thread, and ...

The 'struct interface' is wide open to its users, and each user
fiddles with the fields in a different way.  The cure is, of course,
to hide the implementation details in a source module.  This task
turned out to be harder than it looked at first.

- Patch #1 refactors the makefile a bit (this will be useful in a series to 
follow)
- Patches 2-3 clean up interfaces accepting character strings as parameters.
- Patch #5 introduces the "interface" module.
- Patches 6-23 convert all the users to proper methods, one method at a time.
- Patches 24-27 add create/destroy methods.
- Patch #28 hides the implementation of the struct.
- Patch #29 _finally_ fixes the compiler warning.
- Patch #30 removes left over crud.

Thanks for your review,

Richard Cochran (30):
  Group related objects together within the makefile.
  config: Constify the public interface.
  rtnl: Constify the public interface.
  utils: Constify the posix clock interface.
  Move the network interface into its own header file.
  interface: Introduce an access method for the name field.
  Convert call sites to the proper method for getting interface names.
  interface: Introduce an access method for the time stamping label.
  Convert call sites to the proper method for getting interface labels.
  interface: Introduce a method to get the time stamping information.
  Convert call sites to the proper method for getting time stamp
information.
  interface: Introduce a method to initialize the time stamping label.
  Convert call sites to the proper method for initializing the time
stamping label.
  interface: Introduce a method to set the name.
  Convert call sites to the proper method for setting the name.
  interface: Introduce a method to set the time stamping label.
  Convert call sites to the proper method for setting the time stamping
label.
  interface: Introduce a method to get the PHC index.
  Convert call sites to the proper method for getting the PHC index.
  interface: Introduce a method to test the time stamping information
validity.
  Convert call sites to the proper method for testing time stamp info
validity.
  interface: Introduce a method to test supported time stamping modes.
  Convert call sites to the proper method for testing time stamping
modes.
  interface: Introduce methods to create and destroy instances.
  clock: Use the proper create/destroy API for network interfaces.
  config: Use the proper create/destroy API for network interfaces.
  pmc: Use the proper create/destroy API for network interfaces.
  interface: Hide the implementation details.
  interface: Silence warning from gcc version 8.
  interface: Remove obsolete method.

 clock.c| 61 
 config.c   | 26 --
 config.h   | 19 ++
 interface.c| 78 +
 interface.h| 94 ++
 makefile   | 32 +
 nsm.c  | 18 ++
 pmc_common.c   | 19 +-
 port.c | 59 +--
 port_private.h |  2 +-
 raw.c  |  5 +--
 rtnl.c |  6 ++--
 rtnl.h |  8 +++--
 udp.c  |  4 +--
 udp6.c |  4 +--
 uds.c  |  8 ++---
 util.c |  2 +-
 util.h |  2 +-
 18 files changed, 316 insertions(+), 131 deletions(-)
 create mode 100644 interface.c
 create mode 100644 interface.h

-- 
2.20.1



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel