Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use malloc in fileHandlerImpl #24

Merged
merged 2 commits into from
Sep 28, 2023
Merged

Conversation

clzls
Copy link
Contributor

@clzls clzls commented Sep 28, 2023

As a further work after issue #15 , I think it's our binding developers' responsibility to provide memory safe packages to other developers. So I wrote this very first PR of this project.

  1. string in Nim may contain '\0', not like null-terminating cstring. so
let str: string = "Hi,\0binary"
assert str.len == 10
assert (cstring str).len == 3
assert $(cstring str) == "Hi," # Anything after '\0' is dropped!
assert (cstring str)[4] == 'b'

If content that currHandler returned contains '\0', it was not processed properly. So I turned to use original string's len function. For similar reason, it is required, not optional, to tell WebUI library exact length of the content when content contains nulls.

  1. The memory of temporary cstring object may be GC'ed unexpectedly before it is used by WebUI library. We may use other mechanism to prevent so, but using webui_malloc is encouraged by WebUI document and seems to be a simpler, easier and safer solution.

  2. Return type of proc malloc* is missing. Is there something wrong with binding generation mechanism?

@clzls
Copy link
Contributor Author

clzls commented Sep 28, 2023

Note: Tested using a test fixture that requires a file at a speed of 10 requests per second. It worked fine for several thousands of iterations. Further tests are going to be designed.
(If I make requests much quicker than 10 req/s, for example 1000 req/s, the backend could not keep up the pace.)

setInterval(() => {
  fetch('./getUuid').then(
    resp => resp.text().then(
      txt => {
        let k = document.createElement('span')
        k.innerText = txt + ' ';
        document.getElementById('logs').append(k);
      })
  );
}, 100);

@hassandraga hassandraga merged commit c74224f into webui-dev:main Sep 28, 2023
6 checks passed
@clzls clzls deleted the use_malloc branch October 3, 2023 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants