Bug 49918 - MAX_TRIES and TIMEOUT are hardcoded options; what about making them options?
Summary: MAX_TRIES and TIMEOUT are hardcoded options; what about making them options?
Status: RESOLVED FIXED
Alias: None
Product: libfprint
Classification: Unclassified
Component: fprintd (show other bugs)
Version: unspecified
Hardware: All All
: medium enhancement
Assignee: libfprint-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-14 08:35 UTC by Didier 'OdyX' Raboud
Modified: 2013-03-26 16:18 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Proposed patch to solve this. (2.39 KB, patch)
2012-05-14 08:35 UTC, Didier 'OdyX' Raboud
Details | Splinter Review

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.