Re: [OSS-Tools] [PATCH platsch V4 3/4] platsch: split into platsch and libplatsch

[Philipp Zabel <p.zabel@pengutronix.de>]

On Fr, 2024-06-14 at 11:23 +0200, LI Qingwu wrote:
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  libplatsch.c | 590 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  libplatsch.h |  51 +++++
>  meson.build  |   7 +-
>  platsch.c    | 582 +-------------------------------------------------
>  4 files changed, 652 insertions(+), 578 deletions(-)
>  create mode 100644 libplatsch.c
>  create mode 100644 libplatsch.h
> 
> diff --git a/libplatsch.c b/libplatsch.c
> new file mode 100644
> index 0000000..1d48e0e
> --- /dev/null
> +++ b/libplatsch.c
> @@ -0,0 +1,590 @@
[...]
> +ssize_t readfull(int fd, void *buf, size_t count)
[...]
> +void draw_buffer(struct modeset_dev *dev, const char *dir, const char *base)
[...]
> +void update_display(struct modeset_dev *dev)

Should these be made static?

[...]
> diff --git a/meson.build b/meson.build
> index 0aadbe9..4c2ad7b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1,13 +1,10 @@
>  project('platsch', 'c', version: '2024.06.0')
>  
>  dep_libdrm = dependency('libdrm', version : '>= 2.4.112', required : true)
> -sources = ['platsch.c']
> +sources = ['libplatsch.c']

IMHO this should be:

platsch_sources = [ 'platsch.c' ]
libplasch_sources = [ 'libplatsch.c', 'libplatsch.h' ]

> -# Define the headers
> -headers = ['platsch.h']
>  
> -# Create the platsch executable

Please don't add lines that are removed right away in the next patch.
This belongs in Patch 2.

> -executable('platsch',

What about the platsch executable? It should not be removed.

> +libplatsch = static_library('libplatsch',
>      sources,
>      dependencies: dep_libdrm,
>      install: true,
> diff --git a/platsch.c b/platsch.c
> index 1aaa8d5..917fec0 100644

regards
Philipp