Skip to content

Upload return len - #49

Open
edvardxyz wants to merge 5 commits into
masterfrom
upload_return_len
Open

Upload return len#49
edvardxyz wants to merge 5 commits into
masterfrom
upload_return_len

Conversation

@edvardxyz

@edvardxyz edvardxyz commented Jun 16, 2025

Copy link
Copy Markdown
Contributor

Align upload download API.
Added check for 64 bit address with version 1
Added a param_error.h to start making libparam a more coherent library.
Changed return values to ssize_t, not sure if we want to keep int or make it int64.
This is only necessary if we want to be able to return the count of the full 4GB possible upload and download length supported.
ssize_t would work on 64 bit client, not on 32 bit.
Forcing int64 would fix this

@troelsjessen
troelsjessen requested a review from Lykkeberg June 17, 2025 10:20
Comment thread src/vmem/vmem_client.c
}

int vmem_download(int node, int timeout, uint64_t address, uint32_t length, char * dataout, int version, int use_rdp) {
ssize_t vmem_download(uint16_t node, uint32_t timeout, uint64_t address, char * dataout, uint32_t length, int use_rdp, int version, int verbosity) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are breaking the API signature :-) but if you do, why not implement the verbosity argument as a const? This might help the linker in leaving out the printf() strings from the .const section in FLASH when compiled for the modules and not CSH.

Comment thread src/vmem/vmem_client.c

void vmem_progress(uint32_t total, uint32_t sofar) {
(void) total;
ssize_t vmem_upload(uint16_t node, uint32_t timeout, uint64_t address, const char * datain, uint32_t length, int use_rdp, int version, int verbosity) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like for the vmem_download method I have the same comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants