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

Fix 32bit builds #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix 32bit builds #50

wants to merge 2 commits into from

Conversation

roxell
Copy link
Contributor

@roxell roxell commented Apr 22, 2021

These patches solves the 32-bit builds that was reported a few days back and it fixes a bug.

…he offset

It appears that the previous code cannot possibly work correctly, but I
could not produce any failure without this patch.
However, when building for arm32 bit, the following error shows up:

run_system_call.c: In function 'syscall_splice':
run_system_call.c:3250:31: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 3250 |   result = splice(fd_in_live, (loff_t *) off_in, fd_out_live,
      |                               ^
run_system_call.c:3251:5: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 3251 |     (loff_t *) off_out, len, flags);
      |     ^
cc1: all warnings being treated as errors
make: *** [<builtin>: run_system_call.o] Error 1

The man pages for splice says that splice expects a pointer to the
offset. This is done for ifc.splice. Doing the same for splice.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
When building for arm32 the following error shows up:

run_system_call.c: In function 'cmsg_expect_eq':
run_system_call.c:860:38: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned in
t'} [-Werror=format=]
  860 |      "Bad len in cmsg %d: expected=%lu actual=%lu",
      |                                    ~~^
      |                                      |
      |                                      long unsigned int
      |                                    %u
  861 |      i, ecm->cmsg_len, acm->cmsg_len);
      |         ~~~~~~~~~~~~~

run_system_call.c:3129:6: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'uint64_t' {aka 'long long
 unsigned int'} [-Werror=format=]
 3129 |      "epoll_event->data does not match script: "
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
......
 3133 |      event_live->data.u64);
      |      ~~~~~~~~~~~~~~~~~~~~

To fix these use %zu instead of %lu for size_t, and for the uint64_t use
%PRIu64 instad of %lu.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
@nealcardwell
Copy link
Collaborator

Thanks or the patches!

Regarding the patch:
"net-test: packetdrill: run_system_call: splice expects a pointer to the offset"
Scripts need to be able to pass NULL for the off_in and off_out parameters to the system call. How will they be able to do that after your patch?

Regarding the second patch, "net-test: packetdrill: run_system_call: fix 32 bit builds"; in the commit message:

(1) please document the compile error you received on this line, that motivated changing that one:
event->data.ptr = (void *)epollev_expr->ptr->value.num;

(2) please document that you tested your change by also compiling the resulting code on a 64-bit x86 Linux machine as well, and verifying that this had no warnings or errors.

Thanks!

@roxell
Copy link
Contributor Author

roxell commented May 3, 2021

Thanks or the patches!

Regarding the patch:
"net-test: packetdrill: run_system_call: splice expects a pointer to the offset"
Scripts need to be able to pass NULL for the off_in and off_out parameters to the system call. How will they be able to do that after your patch?

I'm sorry but its not clear to me what the code should do.
I saw that splice and ifc.splice didn't do the same. I assume I have to do something like:
'(loff_t *)(uintptr_t)off_in' same for off_out in both splice and ifc.splice then or what do I miss?

Regarding the second patch, "net-test: packetdrill: run_system_call: fix 32 bit builds"; in the commit message:

(1) please document the compile error you received on this line, that motivated changing that one:
event->data.ptr = (void *)epollev_expr->ptr->value.num;

(2) please document that you tested your change by also compiling the resulting code on a 64-bit x86 Linux machine as well, and verifying that this had no warnings or errors.

Will update "net-test: packetdrill: run_system_call: fix 32 bit builds" according to your suggestions.

Cheers

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