Re: [PATCH i-g-t v3 24/31] tools/intel_guc_logger: link with minimal sub-libraries

Hi Sebastian,

On 2026-07-15 at 14:15:34 +0200, Sebastian Brzezinka wrote:
> intel_guc_logger only needs core logging/assert helpers and debugfs
> access, not the full libigt.so stack. Replace the monolithic include
> with the minimal headers and link against lib_igt_tools_stub.
> 
> The tool previously used igt_simple_main_args() for option parsing, which
> is part of libigt.so and cannot be used without it. Replace it with a
> direct getopt_long() loop. As part of this: add a 'help' long option, a
> usage string header, and a default case that calls exit(1) on unrecognised
> options, behaviour that igt_simple_main_args() provided implicitly.
> 
> ldd before: 35  (ldd | wc -l)
> ldd after:   3  (ldd | wc -l)
> 
> Signed-off-by: Sebastian Brzezinka <[email protected]>
> Acked-by: Ashutosh Dixit <[email protected]>
> Reviewed-by: Krzysztof Niemiec <[email protected]>
> ---
>  tools/intel_guc_logger.c | 21 ++++++++++++++++++---
>  tools/meson.build        |  6 +++++-
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c
> index 629b4e660..3990aef87 100644
> --- a/tools/intel_guc_logger.c
> +++ b/tools/intel_guc_logger.c
> @@ -38,9 +38,12 @@
>  #include <sys/syscall.h>
>  #include <sys/mman.h>
>  #include <assert.h>
> +#include <getopt.h>
>  #include <pthread.h>
>  
> -#include "igt.h"
> +#include "drmtest.h"
> +#include "igt_core.h"
> +#include "igt_debugfs.h"
>  
>  #define MB(x) ((uint64_t)(x) * 1024 * 1024)
>  #ifndef PAGE_SIZE
> @@ -367,6 +370,8 @@ static int parse_options(int opt, int opt_index, void *data)
>  		discard_oldlogs = true;
>  		igt_debug("old/boot-time logs will be discarded\n");
>  		break;
> +	default:
> +		exit(1);
>  	}
>  
>  	return 0;
> @@ -382,10 +387,12 @@ static void process_command_line(int argc, char **argv)
>  		{"polltimeout", required_argument, 0, 'p'},
>  		{"size", required_argument, 0, 's'},
>  		{"discard", no_argument, 0, 'd'},
> +		{"help", no_argument, 0, 'h'},
>  		{ 0, 0, 0, 0 }
>  	};
>  
>  	const char *help =
> +		"Usage: intel_guc_logger [options]\n"
>  		"  -v --verbosity=level   verbosity level of GuC logging (0-3)\n"
>  		"  -o --outputfile=name   name of the output file, including the location, where logs will be stored\n"
>  		"  -b --buffers=num       number of buffers to be maintained on logger side for storing logs\n"
> @@ -394,8 +401,16 @@ static void process_command_line(int argc, char **argv)
>  		"  -s --size=MB           max size of output file in MBs after which logging will be stopped\n"
>  		"  -d --discard           discard the old/boot-time logs before entering into the capture loop\n";
>  
> -	igt_simple_init_parse_opts(&argc, argv, "v:o:b:t:p:s:d", long_options,
> -				   help, parse_options, NULL);
> +	int opt, opt_index = 0;
> +
> +	while ((opt = getopt_long(argc, argv, "v:o:b:t:p:s:dh",
> +				  long_options, &opt_index)) != -1) {
> +		if (opt == 'h') {
> +			printf("%s", help);
> +			exit(0);
> +		}
> +		parse_options(opt, opt_index, NULL);
> +	}
I looked briefly at other programs in /tools directory and many
of them implement their own arg handling (many in similar way:
while loop + switch). It would be nice to have them unified in
a way in the future, but for now:
Reviewed-by: Krzysztof Karas <[email protected]>

>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tools/meson.build b/tools/meson.build
> index 15bd40dec..32a345e9d 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -19,7 +19,6 @@ tools_progs = [
>  	'intel_display_crc',
>  	'intel_display_poller',
>  	'intel_framebuffer_dump',
> -	'intel_guc_logger',
>  	'intel_hdcp',
>  	'intel_infoframes',
>  	'intel_opregion_decode',
> @@ -112,6 +111,11 @@ executable('intel_forcewaked', 'intel_forcewaked.c',
>  	   install_rpath : bindir_rpathdir,
>  	   install : true)
>  
> +executable('intel_guc_logger', 'intel_guc_logger.c',
> +	   dependencies : [lib_igt_tools_stub, libdrm, pthreads],
> +	   install_rpath : bindir_rpathdir,
> +	   install : true)
> +
>  if libudev.found()
>  	intel_dp_compliance_src = [
>  		'intel_dp_compliance.c',
> -- 
> 2.53.0
> 

-- 
Best Regards,
Krzysztof