[app/xauth,2/2] Sort entries from most specific to most generic.

Submitted by Michal Srb on May 31, 2018, 1:12 p.m.

Details

Message ID 20180531131236.6408-3-msrb@suse.com
State New
Headers show
Series "Fix xauth interaction with wildcard entries." ( rev: 1 ) in X.org

Not browsing as part of any series.

Commit Message

Michal Srb May 31, 2018, 1:12 p.m.
There is no point in adding entry or merging lists if a FamilyWild entry would
end in front of any entry, or entry without display number would end in front
of entry with number.

This sorts all entries in order:
  * FamilyWild without display number
  * FamilyWild with display number
  * Other family without display number
  * Other family with display number

The order of the entries in each category is kept.
---
 process.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Patch hide | download patch | download mbox

diff --git a/process.c b/process.c
index d3ea435..f3d6ca4 100644
--- a/process.c
+++ b/process.c
@@ -1170,6 +1170,45 @@  merge_entries(AuthList **firstp, AuthList *second, int *nnewp, int *nreplp)
 
 }
 
+static void
+sort_entries(AuthList **firstp)
+{
+    /* Insert sort, in each pass it removes auth records of certain */
+    /* cathegory from the given list and inserts them into the sorted list. */
+
+    AuthList *sorted = NULL, *sorted_tail = NULL;
+    AuthList *prev, *iter, *next;
+
+    #define SORT_OUT(EXPRESSION) { \
+	prev = NULL; \
+	for (iter = *firstp; iter; iter = next) { \
+	    next = iter->next; \
+	    if (EXPRESSION) { \
+		if (prev) \
+		    prev->next = next; \
+		else \
+		    *firstp = next; \
+		if (sorted_tail == NULL) { \
+		    sorted = sorted_tail = iter; \
+		} else { \
+		    sorted_tail->next = iter; \
+		    sorted_tail = iter; \
+		} \
+		iter->next = NULL; \
+	    } else { \
+		prev = iter; \
+	    } \
+	} \
+    }
+
+    SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length != 0);
+    SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length == 0);
+    SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length != 0);
+    SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length == 0);
+
+    *firstp = sorted;
+}
+
 static Xauth *
 copyAuth(Xauth *auth)
 {
@@ -1508,6 +1547,7 @@  do_merge(const char *inputfilename, int lineno, int argc, const char **argv)
 	  printf ("%d entries read in:  %d new, %d replacement%s\n",
 	  	  nentries, nnew, nrepl, nrepl != 1 ? "s" : "");
 	if (nentries > 0) xauth_modified = True;
+	sort_entries(&xauth_head);
     }
 
     return 0;
@@ -1656,6 +1696,7 @@  do_add(const char *inputfilename, int lineno, int argc, const char **argv)
 	fprintf (stderr, "unable to merge in added record\n");
 	return 1;
     }
+    sort_entries(&xauth_head);
 
     xauth_modified = True;
     return 0;

Comments

> Michal Srb <msrb@suse.com> hat am 31. Mai 2018 um 15:12 geschrieben:
> 
> 
> There is no point in adding entry or merging lists if a FamilyWild entry would
> end in front of any entry, or entry without display number would end in front
> of entry with number.
> 
> This sorts all entries in order:
>   * FamilyWild without display number
>   * FamilyWild with display number
>   * Other family without display number
>   * Other family with display number
> 
> The order of the entries in each category is kept.
> ---
>  process.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/process.c b/process.c
> index d3ea435..f3d6ca4 100644
> --- a/process.c
> +++ b/process.c
> @@ -1170,6 +1170,45 @@ merge_entries(AuthList **firstp, AuthList *second, int
> *nnewp, int *nreplp)
>  
>  }
>  
> +static void
> +sort_entries(AuthList **firstp)
> +{
> +    /* Insert sort, in each pass it removes auth records of certain */
> +    /* cathegory from the given list and inserts them into the sorted list.
> */
> +
> +    AuthList *sorted = NULL, *sorted_tail = NULL;
> +    AuthList *prev, *iter, *next;
> +
> +    #define SORT_OUT(EXPRESSION) { \
> +	prev = NULL; \
> +	for (iter = *firstp; iter; iter = next) { \
> +	    next = iter->next; \
> +	    if (EXPRESSION) { \
> +		if (prev) \
> +		    prev->next = next; \
> +		else \
> +		    *firstp = next; \
> +		if (sorted_tail == NULL) { \
> +		    sorted = sorted_tail = iter; \
> +		} else { \
> +		    sorted_tail->next = iter; \
> +		    sorted_tail = iter; \
> +		} \
> +		iter->next = NULL; \
> +	    } else { \
> +		prev = iter; \
> +	    } \
> +	} \
> +    }
> +
> +    SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length !=
> 0);
> +    SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length ==
> 0);
> +    SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length !=
> 0);
> +    SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length ==
> 0);
> +
> +    *firstp = sorted;
> +}
> +
>  static Xauth *
>  copyAuth(Xauth *auth)
>  {
> @@ -1508,6 +1547,7 @@ do_merge(const char *inputfilename, int lineno, int
> argc, const char **argv)
>  	  printf ("%d entries read in:  %d new, %d replacement%s\n",
>  	  	  nentries, nnew, nrepl, nrepl != 1 ? "s" : "");
>  	if (nentries > 0) xauth_modified = True;
> +	sort_entries(&xauth_head);
>      }
>  
>      return 0;
> @@ -1656,6 +1696,7 @@ do_add(const char *inputfilename, int lineno, int argc,
> const char **argv)
>  	fprintf (stderr, "unable to merge in added record\n");
>  	return 1;
>      }
> +    sort_entries(&xauth_head);
>  
>      xauth_modified = True;
>      return 0;


MMh, so far i understand you do an add and then sort the whole thing.
perhaps it would be more easy to start with a sorted field and insert (add)
a key on the correct location ?

re,
 wh

> -- 
> 2.13.6
> 
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
On úterý 5. června 2018 17:46:05 CEST Walter Harms wrote:
> MMh, so far i understand you do an add and then sort the whole thing.
> perhaps it would be more easy to start with a sorted field and insert (add)
> a key on the correct location ?

That would work if we have the guarantee that the list is sorted to begin 
with. The list will be sorted now if it was created by xauth. But if it was 
made by anything else (e.g. some display manager) it can be in any order. So 
when adding new entry, xauth will re-sort it.

Alternative would be to just insert the new entry in front of the first more 
general entry and ignore any entries that are never used because of their 
position in the list.

I don't have opinion on which is better. Both would work for my usecase.

Michal Srb
> Michal Srb <msrb@suse.com> hat am 6. Juni 2018 um 08:34 geschrieben:
> 
> 
> On úterý 5. června 2018 17:46:05 CEST Walter Harms wrote:
> > MMh, so far i understand you do an add and then sort the whole thing.
> > perhaps it would be more easy to start with a sorted field and insert (add)
> > a key on the correct location ?
> 
> That would work if we have the guarantee that the list is sorted to begin 
> with. The list will be sorted now if it was created by xauth. But if it was 
> made by anything else (e.g. some display manager) it can be in any order. So 
> when adding new entry, xauth will re-sort it.
> 
> Alternative would be to just insert the new entry in front of the first more 
> general entry and ignore any entries that are never used because of their 
> position in the list.
> 
> I don't have opinion on which is better. Both would work for my usecase.
> 

I see your ideas,
I was thinking about a more permanent solution and sofar i see the correct
way would to add the two functions to libXau since i do not see that other
applications would be more clever that xauth.

re,
 wh
On čtvrtek 7. června 2018 14:59:44 CEST Walter Harms wrote:
> I was thinking about a more permanent solution and sofar i see the correct
> way would to add the two functions to libXau since i do not see that other
> applications would be more clever that xauth.

That would be nice, sadly libXau does not provide any functions to manipulate 
the list of xauth records. Just functions for writing and reading of a single 
record, locking and matching.

One option would be to change the semantics of XauGetBestAuthByAddr to return 
the most precise match first, instead of returning the first match of any 
kind. But I don't think changing that behavior is good idea.

Alternatively we can just take the patch #1, so that xauth does overwrite 
records with other random records. I believe that is correct fix. And we can 
ignore this patch #2 that does the sorting. I could probably workaround my 
issue by invoking xauth differently. (I could create 1-item list first 
containing the new record and then merge the original list into it, so the new 
record will end up on top.)

Michal Srb
> Michal Srb <msrb@suse.com> hat am 7. Juni 2018 um 16:01 geschrieben:
> 
> 
> On čtvrtek 7. června 2018 14:59:44 CEST Walter Harms wrote:
> > I was thinking about a more permanent solution and sofar i see the correct
> > way would to add the two functions to libXau since i do not see that other
> > applications would be more clever that xauth.
> 
> That would be nice, sadly libXau does not provide any functions to manipulate 
> the list of xauth records. Just functions for writing and reading of a single 
> record, locking and matching.
> 
> One option would be to change the semantics of XauGetBestAuthByAddr to return 
> the most precise match first, instead of returning the first match of any 
> kind. But I don't think changing that behavior is good idea.
> 
> Alternatively we can just take the patch #1, so that xauth does overwrite 
> records with other random records. I believe that is correct fix. And we can 
> ignore this patch #2 that does the sorting. I could probably workaround my 
> issue by invoking xauth differently. (I could create 1-item list first 
> containing the new record and then merge the original list into it, so the new
> 
> record will end up on top.)
> 
> Michal Srb


I am waiting if someone will complain, if nothing happens until fr. i will see
to add your patch and will mail you for testing.

re,
 wh