Skip to content

fix: add missing return for stat Unix functions #1519

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 26, 2023
Merged

fix: add missing return for stat Unix functions #1519

merged 3 commits into from
Oct 26, 2023

Conversation

@brandonspark
Copy link
Contributor

@brandonspark brandonspark commented Oct 24, 2023 •
edited
Loading

What:

This PR fixes the JS implementation of the caml_unix_lstat_64and caml_unix_stat_64 calls.

Why:

Per the OCaml documentation (https://github.com/ocaml/ocaml/blob/f9371a2ea294f75da793451ef7be5dc69aad6b53/otherlibs/unix/unix_unix.ml#L388), the caml_unix_lstat_64 external function has type signature string -> stats. In particular, it is meant to return a value of type stats.

In the Js_of_ocaml implementation of that external function, however, it returns nothing at all. This results in an undefined, which will then proceed to wreak havoc.

caml_unix_stat_64 is similar.

How:

Added back the return for the r object.

@brandonspark brandonspark changed the title fix: add missing return for caml_unix_lstat_64 fix: add missing return for caml_unix_lstat_64 Oct 24, 2023
@hhugo
Copy link
Member

hhugo commented Oct 24, 2023

Same issue with caml_unix_stat_64, do you mind fixing this as well?

@brandonspark brandonspark changed the title fix: add missing return for caml_unix_lstat_64 fix: add missing return for stat Unix functions Oct 24, 2023
@brandonspark
Copy link
Contributor Author

brandonspark commented Oct 24, 2023

@hhugo done!

@hhugo
Copy link
Member

hhugo commented Oct 25, 2023

Thanks for your contribution. Just missing a changelog entry

@brandonspark
Copy link
Contributor Author

brandonspark commented Oct 25, 2023

ah, should have read the CONTRIBUTING.md. Added now! @hhugo

@hhugo hhugo force-pushed the fix-caml-unix-lstat-64-impl branch from 8dbe393 to 0f314ea Compare October 26, 2023 08:50
@hhugo hhugo merged commit 3954ace into ocsigen:master Oct 26, 2023
This was referenced Dec 1, 2023
Closed
Closed
hhugo pushed a commit to hhugo/opam-repository that referenced this pull request Dec 4, 2023
[new release] js_of_ocaml (7 packages) (5.5.1)
295c487 
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
@hhugo hhugo mentioned this pull request Dec 5, 2023
Merged
mseri pushed a commit to ocaml/opam-repository that referenced this pull request Dec 6, 2023
[new release] js_of_ocaml (7 packages) (5.5.2)
72567c0 
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@nberth
[new release] js_of_ocaml (7 packages) (5.5.2)
3b3cc89 
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@brandonspark @hhugo