Build warning: { toolkit/components/url-classifier/Classifier.cpp:802:66: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] } The code in question: 778 nsresult 779 Classifier::ReadNoiseEntries(const Prefix& aPrefix, 780 const nsACString& aTableName, 781 int32_t aCount, 782 PrefixArray* aNoiseEntries) 783 { 784 LookupCache *cache = GetLookupCache(aTableName); 785 if (!cache) { 786 return NS_ERROR_FAILURE; 787 } 788 789 nsTArray<uint32_t> prefixes; 790 nsresult rv = cache->GetPrefixes(&prefixes); 791 NS_ENSURE_SUCCESS(rv, rv); 792 793 int32_t idx = prefixes.BinaryIndexOf(aPrefix.ToUint32()); 794 795 if (idx == nsTArray<uint32_t>::NoIndex) { 796 NS_WARNING("Could not find prefix in PrefixSet during noise lookup"); 797 return NS_ERROR_FAILURE; 798 } 799 800 idx -= idx % aCount; 801 802 for (int32_t i = 0; (i < aCount) && ((idx+i) < prefixes.Length()); i++) { 803 Prefix newPref; 804 newPref.FromUint32(prefixes[idx+i]); 805 if (newPref != aPrefix) { 806 aNoiseEntries->AppendElement(newPref); 807 } 808 } https://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/Classifier.cpp#778 Here, both |i| and |idx| are compared to unsigned values, and IIUC they also can only take non-negative values -- so we should declare them as unsigned.

(In reply to Daniel Holbert [:dholbert] from comment #0) > Here, both |i| and |idx| are compared to unsigned values, and IIUC they also > can only take non-negative values -- so we should declare them as unsigned. So, this is slightly-complicated by the fact that 'aCount' is signed, and it's compared to both 'i' (in the loop condition) and it's used in an operation with 'idx' (at line 800, for the modulus operation). Luckily, 'aCount' traces back to something that can be known-to-be-nonnegative, so we can just change it (and the variable that feeds into it in the chain of callers) to be unsigned as well. I'm posting a patch to do that, first, and then I'll post a second patch to convert 'i' and 'idx' to fix the warning.

The |aCount| variable traces back to |gethashNoise|, and we already bounds-check it in AddNoise() (we return early if it's less than 1), so it's already known-to-be-nonnegative where it's actually used. This patch converts it to an unsigned value everywhere. Also, at the point where we read it from a (user-controlled) pref, this patch makes us use the default value (4) if the pref has a (nonsensical) negative value.

...and this fixes the actual variables 'i' and 'idx' mentioned in comment 0. This patch fixes the build warning.

https://hg.mozilla.org/integration/mozilla-inbound/rev/58af88ca07be https://hg.mozilla.org/integration/mozilla-inbound/rev/6a17b673aeca

https://hg.mozilla.org/mozilla-central/rev/58af88ca07be https://hg.mozilla.org/mozilla-central/rev/6a17b673aeca