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

Missing malloc #15

Open
roadelou opened this issue Feb 27, 2021 · 2 comments
Open

Missing malloc #15

roadelou opened this issue Feb 27, 2021 · 2 comments

Comments

@roadelou
Copy link

Dear all,

As of commit 252cecb, the malloc function is not provided by the pulp-runtime. Is that a choice or an omission?

Steps to reproduce

  1. Clone the repository: git clone https://github.com/pulp-platform/pulp-runtime.git
  2. Got to the root of the repository: cd pulp-runtime
  3. Search for malloc with grep: grep -R "void \*malloc"

This should yield a single result, which is the declaration of malloc in the stdlib header file.

Alternatively, one can:

  1. Install the pulp-builder toolchain, which in turns installs pulp-runtime
  2. Write a project which uses malloc
  3. Compile it. The compiler won't complain because malloc is declared in a header, however the linker will fail because no implementation for the function is provided

Additional Information

The function was provided in the pulp-rt repository, see io.c, and there is a low-level allocator in the current pulp-runtime, see the pos_alloc function in the kernel.

Note that the free function is also missing.

Request for Change

Would it be possible to provide an implementation for malloc if it was forgotten? Should the removal be intentional, could the declaration be also removed from the stdlib header? That way, the compiler will warn a user about the missing malloc, instead of yielding a more obscure linker error.

On the other hand, if there is a reason for keeping the situation as it is now, would it possible to provide the explanation either as a comment in the stdlib header, or maybe in the README of the repository?

Regardless, thank you all for the work on this great project!

@bluewww
Copy link
Collaborator

bluewww commented Mar 7, 2021

pulp-runtime indeed does not provide a full c stdlib. If you need that I recommend you use nano-newlib or something else.

I guess adding malloc and free makes sense.

@roadelou
Copy link
Author

roadelou commented Mar 7, 2021

Thank you for your answer.

Indeed, thank you for your suggestions. For our application we don't need a full stdlib, and I would understand if you want to keep pulp-runtime minimal and thus not include malloc and free. We could probably work around that by using pos_alloc and pos_free directly in our case.

Regardless, I have sent a pull request #16 on the GitHub repository, that way you can judge how much new code would be needed for malloc and if it should be included in the basic runtime.

If you decide not to include malloc and free feel free to reject the Pull Request, but in that case I would advise also removing them from the stdlib header to get a clear compilation error.

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

No branches or pull requests

2 participants