You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/06/22 07:14:00 UTC

[GitHub] [lucene] dweiss commented on a diff in pull request #970: LUCENE-10607: Fix potential integer overflow in maxArcs computions

dweiss commented on code in PR #970:
URL: https://github.com/apache/lucene/pull/970#discussion_r903362582


##########
lucene/suggest/src/java/org/apache/lucene/search/suggest/document/NRTSuggesterBuilder.java:
##########
@@ -124,11 +124,15 @@ public boolean store(DataOutput output) throws IOException {
    * <p>TODO: is there a better way to make the fst built to be more TopNSearcher friendly?
    */
   private static int maxNumArcsForDedupByte(int currentNumDedupBytes) {
+    // when currentNumDedupBytes larger than 12,the result is definitely larger than 255;

Review Comment:
   ```suggestion
       // when currentNumDedupBytes is larger than 12, the result is definitely larger than 255
       // (avoid potential int overflow).
   ```



##########
lucene/suggest/src/java/org/apache/lucene/search/suggest/document/NRTSuggesterBuilder.java:
##########
@@ -124,11 +124,15 @@ public boolean store(DataOutput output) throws IOException {
    * <p>TODO: is there a better way to make the fst built to be more TopNSearcher friendly?
    */
   private static int maxNumArcsForDedupByte(int currentNumDedupBytes) {
+    // when currentNumDedupBytes larger than 12,the result is definitely larger than 255;

Review Comment:
   For currentNumDedupBytes = 11, maxArcs = (1 + 11 * 2) * 11 = 253. While this is correct I think it'd be simpler to read the code as:
   ```
   long maxArcs = 2 * currentNumDedupBytes + 1;
   if (currentNumDedupBytes > 5) {
     maxArcs *= currentNumDedupBytes;
   }
   return (int) Math.min(maxArcs, 255);
   ```
   then the cap is explicit?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org