On 9/11/24 14:36, Jakub Palacky wrote:
> When libbsd is available, use the preferred readpassphrase() function isntead of getpass()
> as the getpass() function has been marked as obsolete and shouldnt be used
>
> Signed-off-by: Jakub Palacky <jpalacky@xxxxxxxxxx>
> ---
> Changes in v2:
> - Fix possible memory leak of g_new0
> - Use PASS_MAX for max password length
> - Set PASS_MAX to 1024 if not defined
>
> meson.build | 6 ++++++
> src/meson.build | 1 +
> src/util/virutil.c | 17 +++++++++++++++++
> 3 files changed, 24 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 297fbfae48..699a65c7e8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -954,6 +954,11 @@ if blkid_dep.found()
> conf.set('WITH_BLKID', 1)
> endif
>
> +bsd_dep = dependency('libbsd', required: false)
> +if bsd_dep.found()
> + conf.set('WITH_LIBBSD', 1)
> +endif
> +
I know we have plenty of variables that reference a library but lack
'lib' prefix in their name. For example ..
> capng_dep = dependency('libcap-ng', required: get_option('capng'))
.. this one ^^^. BUT, IMO 'bsd' is a bit specific since it's ambiguous.
String may refer to Berkley Software Distribution (BSD) (an operating
system), or libbsd in this case (a library that implements some
functions BSD offers).
Therefore, I'd prefer if this was named 'libbsd_dep' to make this explicit.
> if capng_dep.found()
> conf.set('WITH_CAPNG', 1)
> @@ -2335,6 +2340,7 @@ libs_summary = {
> 'dlopen': dlopen_dep.found(),
> 'fuse': fuse_dep.found(),
> 'glusterfs': glusterfs_dep.found(),
> + 'libbsd': bsd_dep.found(),
> 'libiscsi': libiscsi_dep.found(),
> 'libkvm': libkvm_dep.found(),
> 'libnbd': libnbd_dep.found(),
> diff --git a/src/meson.build b/src/meson.build
> index 8cce42c7ad..30ae34646e 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -9,6 +9,7 @@ src_dep = declare_dependency(
> dependencies: [
> glib_dep,
> libxml_dep,
> + bsd_dep,
Almost. In the end we want libvirt.so to link with libbsd.so BUT, from
src/util a static library is linked. And if we had a binary that links
with the static library but not libvirt.so it could not link.
IOW - this change must be done in src/util/meson.build.
> ],
> include_directories: [
> libvirt_inc,
The rest looks good. I'm fixing both small nits and merging.
Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
Congratulations on your first libvirt contribution!
Michal
