Ensure that entries always have at least one specific subkey to match.

The common_subkey logic depends on this. Before this commit, if an
entry only had common and negative subkeys, it never got registered
as a condidate and dcs_state_remove_candidate could fail an assertion.

src/gen_dcs_classifier.pl

@@ -802,28 +802,56 @@ sub process_entry($) {
     my $initmask = 0;
 
     my @sks = (@{$entry->[ENTRY_SUBKEYS]});
+
+    goto nosubkeys unless(scalar(@sks));
+
+    my %sks_common;
+    my %sks_negative;
+    my %sks_positive;
+
+    foreach my $subkey (@sks) {
+	if (exists($common_subkeys{$subkey})) {
+	    $sks_common{$subkey} = 1;
+	} elsif ($subkey =~ /^!(.*)/o) {
+	    $subkey =~ s/^!//;
+	    $sks_negative{$subkey} = 1;
+	} else {
+	    $sks_positive{$subkey} = 1;
+	}
+    }
+
+    # we need at least one positive subkey, otherwise the
+    # common_matchmask and candidate logic will fail
+    if (scalar(keys %sks_positive) == 0) {
+	my @common = keys %sks_common;
+
+	die 'neither positive nor common subkeys for key '.${$entry->[ENTRY_KEY]}
+	    unless (scalar(@common) > 0);
+
+	# choose the common subkey with the lowest count
+	# (which is the one with the highest value)
+	@common = sort { $common_subkeys{$a} <=> $common_subkeys{$a} } @common
+	    unless (scalar(@common) == 1);
+
+	$sks_positive{$common[0]} = 1;
+	delete $sks_common{$common[0]};
+    }
+
     while (my $subkey = shift @sks) {
 	assert($entry_subkey < MAX_SUBKEY_PER_ENTRY);
 
 	my @se;
 
-	# for common subkeys we just mark the appropriate bit to be
-	# required, except when this is the last subkey and all hae
-	# been common so far. In that case we must not use the common
-	# subkey logic but rather have the entry marked explicitly to
-	# get it considered as a candidate
-	if (exists($common_subkeys{$subkey}) &&
-	    (! (($entry_subkey == 0) &&
-		(scalar(@sks) == 0)))) {
+	if ($sks_common{$subkey}) {
 	    $common_matchmask |= $common_subkeys{$subkey};
 	} else {
 	    my $e;
-	    if ($subkey =~ /^!(.*)/o) {
-		$subkey =~ s/^!//;
+	    if ($sks_negative{$subkey}) {
 		$initmask |= (1<<$entry_subkey);
 		$matchmask |= (1<<$entry_subkey);
 		$e = 0 - $entry->[ENTRY_INDEX];
 	    } else {
+		assert($sks_positive{$subkey});
 		$matchmask |= (1<<$entry_subkey);
 		$e = $entry->[ENTRY_INDEX];
 	    }
@@ -851,6 +879,8 @@ sub process_entry($) {
 	    $subkey_id++;
 	}
     }
+
+  nosubkeys:
     $entry->[ENTRY_COMMON_MATCHMASK]	= $common_matchmask;
     $entry->[ENTRY_MATCHMASK]		= $matchmask;
     $entry->[ENTRY_INITMASK]		= $initmask;
@@ -1186,6 +1216,14 @@ dcs_state_add_candidate(struct dcs_matchstate *state, dcs_entry_id_t cand) {
 	state->n_candidates++;
 }
 
+#define dcs_entry_positive(entry_index) (dcs_matchstate_init.matchmask[entry_index] == 0)
+
+/* have got no common_matchmask or all entries hit */
+#define dcs_entry_common_match(state, entry_index)					\
+	    (dcs_entry[entry_index].common_matchmask == 0) ||				\
+	    ((state->common_matchmask & dcs_entry[entry_index].common_matchmask) == 	\
+	    dcs_entry[entry_index].common_matchmask)
+
 static inline void
 dcs_state_remove_candidate(struct dcs_matchstate *state, dcs_entry_id_t cand) {
 	dcs_entry_id_t i, j;
@@ -1202,8 +1240,12 @@ dcs_state_remove_candidate(struct dcs_matchstate *state, dcs_entry_id_t cand) {
 			return;
 		}
 	}
-
-	/* must not happen that we remove something not a candidate */
+	/*
+	 * it must not happen that we remove something not a candidate
+	 * special cases must be handled in dcs_entry gen code
+	 * (like only common and negative subkeys as in
+	 * iphone*!ipad*!ipod*!freenetmail)
+	 */
 	assert("removed something not a candidate" == NULL);
 }
 
@@ -1214,9 +1256,7 @@ dcs_state_eval_candidates(struct dcs_matchstate *state) {
 	for (i = 0; i  < state->n_candidates; i++) {
 		dcs_entry_id_t entry_index = state->candidates[i];
 
-		if ((dcs_entry[entry_index].common_matchmask == 0) ||
-		    ((state->common_matchmask & dcs_entry[entry_index].common_matchmask) ==
-		    dcs_entry[entry_index].common_matchmask)) {
+		if (dcs_entry_common_match(state, entry_index)) {
 			if ((state->min_match_entry == 0) ||
 			    (entry_index < state->min_match_entry))
 				state->min_match_entry = entry_index;
@@ -1267,7 +1307,6 @@ dcs_register_subkey_match(struct dcs_matchstate *state, dcs_subkey_id_t subkey_i
 		if (entry_index < 0) {
 			entry_index = 0 - entry_index;
 
-			/* if this was a candidate before, remove it */
 			if (state->matchmask[entry_index] == dcs_entry[entry_index].matchmask) {
 				dcs_state_remove_candidate(state, entry_index);
 				P_SK_MATCH("removed candidate index %d id %d this 0x%x mask 0x%x need 0x%x\n",
@@ -1294,22 +1333,17 @@ dcs_register_subkey_match(struct dcs_matchstate *state, dcs_subkey_id_t subkey_i
 		 * - if mask ok and common mask ok, we got a match
 		 * - if mask ok but not common mask, make it a candidate
 		 *
-		 * negatives are more complicates:
+		 * negatives are more complicated:
 		 * - if we see the negative subkey before the last match, the matchmasks
-		 *   of state and entry will never be equal
+		 *   of state and entry will never get eqal -> ok
 		 * - otherwise we cannot know that it is a match before we have seen the
 		 *   whole string, so make it a candidate for final check
 		 *   it is sufficient to check the common mask in the final check
 		 */
 
 		if (state->matchmask[entry_index] == dcs_entry[entry_index].matchmask) {
-			if (dcs_matchstate_init.matchmask[entry_index] == 0) {
-				/* entry with only positive matches */
-				if ((dcs_entry[entry_index].common_matchmask == 0) ||
-				    ((state->common_matchmask & dcs_entry[entry_index].common_matchmask) ==
-				    dcs_entry[entry_index].common_matchmask)) {
-					/* have got no common_matchmask or all entries hit */
-
+			if (dcs_entry_positive(entry_index)) {
+				if (dcs_entry_common_match(state, entry_index)) {
 					if ((state->min_match_entry == 0) ||
 					    (entry_index < state->min_match_entry))
 						state->min_match_entry = entry_index;