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@redhat.com>
---
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
+
 capng_dep = dependency('libcap-ng', required: get_option('capng'))
 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,
   ],
   include_directories: [
     libvirt_inc,
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 6c89a48e51..bf6008fdfb 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -31,6 +31,10 @@
 # include <conio.h>
 #endif /* WIN32 */
 
+#ifdef WITH_LIBBSD
+# include <bsd/readpassphrase.h>
+#endif
+
 #ifdef __linux__
 # include <sys/sysmacros.h>
 #endif
@@ -1773,6 +1777,19 @@ char *virGetPassword(void)
     }
 
     return g_string_free(pw, FALSE);
+#elif WITH_LIBBSD /* !WIN32 */
+# ifndef PASS_MAX
+#  define PASS_MAX 1024
+# endif
+    char *pass = NULL;
+    g_autofree char *buffer = g_new0(char, PASS_MAX);
+
+    pass = readpassphrase("", buffer, PASS_MAX, 0);
+    if (pass == NULL) {
+        return NULL;
+    }
+
+    return g_steal_pointer(&buffer);
 #else /* !WIN32 */
     return g_strdup(getpass(""));
 #endif /* ! WIN32 */
-- 
2.46.0

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@redhat.com>
> ---
> 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@redhat.com>

Congratulations on your first libvirt contribution!

Michal