Bug 49918

Summary: MAX_TRIES and TIMEOUT are hardcoded options; what about making them options?
Product: libfprint Reporter: Didier 'OdyX' Raboud <odyx>
Component: fprintdAssignee: libfprint-bugs
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: medium    
Version: unspecified   
Hardware: All   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Proposed patch to solve this.

Description Didier 'OdyX' Raboud 2012-05-14 08:35:46 UTC 
Created attachment 61630 [details] [review]
Proposed patch to solve this.

Hi,

while discussing the inclusion of fprintd in Debian (see http://bugs.debian.org/502138 ), I noticed that MAX_TRIES and TIMEOUT are not easily configureable and I think it would be worthwile to be able to configure them.

Attached is a proposed patch (please forgive the roughness of my C/Gtk skills) that adds the possibility to add max_tries=1 timeout=15 to the pam_fprintd.so line in PAM config.

Please consider; cheers,

OdyX
Comment 1 Bastien Nocera 2013-03-26 11:39:23 UTC 
Comment on attachment 61630 [details] [review]
Proposed patch to solve this.

Review of attachment 61630 [details] [review]:
-----------------------------------------------------------------

::: pam/pam_fprintd.c
@@ +435,5 @@
> +			if(g_str_equal (argv[i], "debug")) {
> +				g_message ("debug on");
> +				debug = TRUE;
> +			}
> +			else if (strncmp(argv[i], "max_tries=", 10) == 0 && strlen(argv[i]) == 11){

This should be:
#define MAX_TRIES_MATCH
and use strlen (MAX_TRIES_MATCH) in the code. Would avoid possible crashes if we change the option name.

@@ +438,5 @@
> +			}
> +			else if (strncmp(argv[i], "max_tries=", 10) == 0 && strlen(argv[i]) == 11){
> +				char max_tries_l[1];
> +				strncpy(max_tries_l,argv[i] + 10,1);
> +				sscanf(max_tries_l,"%d", &max_tries);

Eek.

max_tries = atoi(argv[1] + strlen (MAX_TRIES_MATCH);

@@ +441,5 @@
> +				strncpy(max_tries_l,argv[i] + 10,1);
> +				sscanf(max_tries_l,"%d", &max_tries);
> +				D(pamh, "max_tries specified as: %d",max_tries);
> +			}
> +			else if (strncmp(argv[i], "timeout=", 8) == 0 && strlen(argv[i]) <= 10){

Same comments here.
Comment 2 Bastien Nocera 2013-03-26 11:46:36 UTC 
Pushed with those fixes.

commit 9c99e5cd59fc942ea0d5f5d1263424eaf78c27b4
Author: Didier Raboud <odyx@debian.org>
Date:   Mon May 14 17:29:56 2012 +0200

    pam: Make max_tries and timeout arguments.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=49918
Comment 3 Didier 'OdyX' Raboud 2013-03-26 16:18:37 UTC 
Great! Thanks for the review and inclusion!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.