Re: [quicwg/base-drafts] [qpack] Overhaul the pseudocode (#3577)

Mike Bishop <notifications@github.com> Thu, 16 April 2020 20:42 UTC

Date: Thu, 16 Apr 2020 13:42:56 -0700
From: Mike Bishop <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK4MGKEDOYC6Q4NMSQV4USSFBEVBNHHCHNWTMM@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3577/review/394978073@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3577@github.com>
References: <quicwg/base-drafts/pull/3577@github.com>
Subject: Re: [quicwg/base-drafts] [qpack] Overhaul the pseudocode (#3577)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5e98c35012fce_546b3fe49d0cd96c78939"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/PzTWta9m-zgC1BroLM09NfJWdjc>

@MikeBishop requested changes on this pull request.

Looks good.  Your build errors are due to line-length; I think this should fix them.

> +    # getNameIndex attempts to find an index for this header's name.
+    # If one exists, it returns the name's (absolute) index and a

```suggestion
    # getNameIndex searches for this header's name in the table.
    # If an entry matches, it returns the (absolute) index and a
```

>      if shouldIndex(header) and dynamicTable.canIndex(header):
-      encodeLiteralWithIncrementalIndex(controlBuffer, nameIdx,
-                                        header)
+      encodeInsert(encoderBuffer, nameIndex, isStaticName, header)

```suggestion
    encodeInsert(
        encoderBuffer, nameIndex, isStaticName, header
    )
```

> +  wireRIC =
+    (requiredInsertCount % (2 * getMaxEntries(maxTableCapacity))) + 1;

```suggestion
  wireRIC = (
    requiredInsertCount
    % (2 * getMaxEntries(maxTableCapacity))
  ) + 1
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3577#pullrequestreview-394978073

[quicwg/base-drafts] [qpack] Overhaul the pseudoc… afrind
Re: [quicwg/base-drafts] [qpack] Overhaul the pse… Martin Thomson
Re: [quicwg/base-drafts] [qpack] Overhaul the pse… Kazu Yamamoto
Re: [quicwg/base-drafts] [qpack] Overhaul the pse… Mike Bishop
Re: [quicwg/base-drafts] [qpack] Overhaul the pse… afrind
Re: [quicwg/base-drafts] [qpack] Overhaul the pse… afrind