Provided changes to enable building of this file with both clang and … #1527

dzobbe · 2022-09-27T21:09:10Z

…clang++

loganek

Having #ifdef __cplusplus in .c files is a bi unusual; could you tell us about your usecase and explain why C file is compiled with C++ compiler?

dzobbe · 2022-09-28T21:37:29Z

We are running WolfSSL into Wasm (look here -> https://github.com/JamesMenetrey/wolfssl-examples/tree/wasm/Wasm) and at the same time I need some features from C++.

wenyongh · 2022-09-29T07:47:37Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c

+    __wasi_addr_t wasi_addr {} ;
+#else
    __wasi_addr_t wasi_addr = { 0 };
+#endif


@dzobbe Shall we use memset to initialize it here so as to avoid introducing __cplusplus?

__wasi_addr_t wasi_addr; ... memset(&wasi_addr, 0, sizeof(wasi_addr));

wenyongh · 2022-09-29T07:47:48Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c

+    __wasi_addr_t wasi_addr {} ;
+#else
    __wasi_addr_t wasi_addr = { 0 };
+#endif


Same as above

wenyongh · 2022-09-29T07:50:16Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c


    switch (optname) {
-        case SO_RCVTIMEO:
+        case SO_RCVTIMEO: {


Should put { to the next line, or CI code guideline check will report error:

case SO_RCVTIMEO: { assert(optlen == sizeof(struct timeval)); timeout_us = timeval_to_time_us(*(struct timeval *)optval); error = __wasi_sock_set_recv_timeout(sockfd, timeout_us); HANDLE_ERROR(error); return error; }

dzobbe · 2022-09-29T08:53:23Z

Done. Thank you.

wenyongh · 2022-09-29T09:19:13Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c

 {
-    __wasi_addr_t wasi_addr = { 0 };
+    __wasi_addr_t wasi_addr;
+    memset(&wasi_addr, 0, sizeof(wasi_addr));


Please put memset after line 144 (had better make it a line with an empty line above and below), here it is C source code, we usually align with the C coding style.

wenyongh · 2022-09-29T09:19:25Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c

 {
-    __wasi_addr_t wasi_addr = { 0 };
+    __wasi_addr_t wasi_addr;
+    memset(&wasi_addr, 0, sizeof(wasi_addr));


Same as above.

dzobbe · 2022-09-29T09:31:02Z

Take a look now

wenyongh · 2022-09-29T10:02:26Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c

    }

-    aibuf_res = calloc(1, addr_info_size * sizeof(struct aibuf));
+    aibuf_res = (aibuf *) calloc(1, addr_info_size * sizeof(struct aibuf));


Should be (struct aibuf *), or compilation error occurs in Ubuntu.

this is done

Yes, and please help format the coding style by using clang-format-12:
clang-format-12 -i --style=file <this file>

wenyongh · 2022-09-29T10:05:16Z

Take a look now

@dzobbe One comment from me.

And also could you please use clang-format-12 to format the code:
Refer to https://github.com/bytecodealliance/wasm-micro-runtime/actions/runs/3150185668/jobs/5122933217:

Use clang-format-[12](https://github.com/bytecodealliance/wasm-micro-runtime/actions/runs/3150185668/jobs/5122933217#step:3:13) or git-clang-format-12 to check code format of
the PR, with a commit range specified. It is required to format the
code before committing the PR, or it might fail to pass the CI check:

1. Install clang-format-12.0.0
Normally we can install it by `sudo apt-get install clang-format-12`,
or download the `clang+llvm-12.0.0-xxx-tar.xz` package from
  https://github.com/llvm/llvm-project/releases/tag/llvmorg-12.0.0
and install it

2. Format the C/C++ source file
``` shell
cd path/to/wamr/root
clang-format-12 --style file -i path/to/file
```

wenyongh · 2022-09-29T10:30:02Z

core/iwasm/libraries/lib-socket/src/wasi/wasi_socket_ext.c

    memset(&wasi_addr, 0, sizeof(wasi_addr));

-    error = __wasi_sock_accept(sockfd, 0, &new_sockfd);
+    error = __wasi_sock_accept(sockfd, &new_sockfd);


Should not change this line, refer to: #1531

wenyongh · 2022-09-29T10:45:06Z

LGTM

…lliance#1527) Enable to run WolfSSL into wasm and need some features from C++: https://github.com/JamesMenetrey/wolfssl-examples/tree/wasm/Wasm

Provided changes to enable building of this file with both clang and …

5aa1bd5

…clang++

loganek reviewed Sep 27, 2022

View reviewed changes

wenyongh reviewed Sep 29, 2022

View reviewed changes

Code format changes and usage of memset

2bf5184

wenyongh reviewed Sep 29, 2022

View reviewed changes

dzobbe added 2 commits September 29, 2022 11:28

Changes for coding style

d855354

Merge branch 'main' into patch-4

4da261c

wenyongh reviewed Sep 29, 2022

View reviewed changes

dzobbe added 2 commits September 29, 2022 12:05

Added struct to casting

dfcb9fb

formatted using clang-format-12

621014d

wenyongh reviewed Sep 29, 2022

View reviewed changes

Update wasi_socket_ext.c

2396ce9

loganek approved these changes Sep 29, 2022

View reviewed changes

wenyongh merged commit 3fad613 into bytecodealliance:main Sep 29, 2022

Provided changes to enable building of this file with both clang and … #1527

Provided changes to enable building of this file with both clang and … #1527

Uh oh!

Conversation

dzobbe commented Sep 27, 2022

Uh oh!

loganek left a comment

Choose a reason for hiding this comment

Uh oh!

dzobbe commented Sep 28, 2022

Uh oh!

wenyongh Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzobbe commented Sep 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzobbe commented Sep 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wenyongh commented Sep 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wenyongh commented Sep 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wenyongh Sep 29, 2022 •

edited

Loading