-
Notifications
You must be signed in to change notification settings - Fork 149
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
Ver32C return code #1332
Comments
Hi David,
Correct! Thank you. If you want to get credit for this could you create a
pull request with the change? Otherwise, I can change it but then your
obervation gets lost.
Andy
…On Fri, 20 Nov 2020, David Smith wrote:
In XrdOss/XrdOss.cc (current head 8db3054), XrdOssDF::pgWrite:
```
228: if (!XrdOucCRC::Ver32C((void *)buffer, wrlen, csvec, valcs))
229: return -EDOM;
230: }
```
There are several overloaded versions of Ver32C, but I believe this one returns the index of the first mismatching checksum, so the check on line 228 is not general enough to catch all mismatches, probably it should be >= 0.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#1332
|
Hi Andy, OK. I'll open a PR (in a few minutes) - not because it's an issue of wanting credit, but I guess it's a better tracked process, a place to review before merge etc. David |
Thanks David. Actually, I've already reviewed it and you are correct
(assuming the patch will be the expected one). I am big on giving credit
where it's due. In github this is the only way of doing it publicly which
I am a big fan of. Anyone who corrects a bug should be the one that
gets credit for it! Yes, for more involved patches, this is the only way
of doing a review but it is still relevant to IP recognition.
Andy
…On Fri, 20 Nov 2020, David Smith wrote:
Hi Andy,
OK. I'll open a PR (in a few minutes) - not because it's an issue of wanting credit, but I guess it's a better tracked process, a place to review before merge etc.
David
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#1332 (comment)
|
Opened PR #1333. Will close this issue. Thanks, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In XrdOss/XrdOss.cc (current head 8db3054), XrdOssDF::pgWrite:
There are several overloaded versions of Ver32C, but I believe this one returns the index of the first mismatching checksum, so the check on line 228 is not general enough to catch all mismatches, probably it should be >= 0.
The text was updated successfully, but these errors were encountered: