Skip to content

hyper: fix a progress upload counter bug#11780

Closed
nnethercote wants to merge 1 commit intocurl:masterfrom
nnethercote:hyper-progress-tweak
Closed

hyper: fix a progress upload counter bug#11780
nnethercote wants to merge 1 commit intocurl:masterfrom
nnethercote:hyper-progress-tweak

Conversation

@nnethercote
Copy link
Contributor

Curl_pgrsSetUploadCounter should be a passed a total count, not an increment.

This changes the failing diff for test 579 with hyper from this:

 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 29 out of 0[LF]

to this:

 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]

Presumably a step in the right direction.

@github-actions github-actions bot added the Hyper label Sep 1, 2023
`Curl_pgrsSetUploadCounter` should be a passed a total count, not an
increment.

This changes the failing diff for test 579 with hyper from this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 29 out of 0[LF]
```
to this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]
```
Presumably a step in the right direction.
@nnethercote nnethercote changed the title hyper: fix a progress upload counter bug. hyper: fix a progress upload counter bug Sep 1, 2023
@nnethercote
Copy link
Contributor Author

 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]

I more or less understand the differences here now. The test has this data that gets sent:

static const char * const post[]={
  "one",
  "two",
  "three",
  "and a final longer crap: four",
  NULL
};

Vanilla curl gets an update for each entry, which is why there are five entries after the initial UL 0. The length of each update is the length of the string, plus another 5 or 6 chars for a hexlen and some EOL chars. That's how we end up with (3+5) + (3+5) + (5+5) + (29+6) + (0+5) = 66 bytes.

curl+hyper ends up doing things slightly differently: it doesn't include the hexlen/EOL chars, so we end up with 3 + 3 + 5 + 29 + 0 = 40 chars. Also, it doesn't call Curl_pgrsUpdate between each Curl_pgrsSetUploadCounter, so we get a single update at the end instead of multiple updates.

I don't know if these differences are important. Maybe it's worth having different expected output for the hyper build?

@bagder bagder closed this in 73f4ef5 Sep 1, 2023
@bagder
Copy link
Member

bagder commented Sep 1, 2023

Thanks!

ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
`Curl_pgrsSetUploadCounter` should be a passed a total count, not an
increment.

This changes the failing diff for test 579 with hyper from this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 29 out of 0[LF]
```
to this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]
```
Presumably a step in the right direction.

Closes curl#11780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants