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

check osdp close file status (according prototype function comments) #124

Merged
merged 2 commits into from
Sep 3, 2023

Conversation

sgouraud
Copy link

could be usefull if any error during file closing

@sidcha
Copy link
Member

sidcha commented Jun 17, 2023

I conciously chose to ignore this error, here is my reasoning:

  • this close failure is a local issue, the OSDP peer does not need to know about it.
  • all bytes of the file has been sent to the peer and the peer has probably closed it's FD already so don't make sense to ask it to discard the contents due to this issue.

On the other hand, we can have the log message. What do you think?

@sgouraud
Copy link
Author

File closing error should significate data corruption on PD, which could need a full file data transfert again . By the way, I don't see any error on osdp_FTSTAT reply which should explain this file's closing error back to ACU :
-1= abort file transfer
-2= unrecognized file contents
-3=f ile data unacceptable (malformed)

If you think this error could be ignore, the comments above should be remove : https://github.com/goToMain/libosdp/blob/master/include/osdp.h#L1093

Copy link
Member

@sidcha sidcha left a comment

Choose a reason for hiding this comment

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

Reasoning sounds good to me. Thanks for the PR.

@sidcha sidcha merged commit 311bc57 into goToMain:master Sep 3, 2023
9 checks passed
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