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

Optimize cmsOpenIOhandlerFromMem() in 'w' mode #443

Merged
merged 1 commit into from
Apr 17, 2024
Merged

Conversation

x2018
Copy link
Contributor

@x2018 x2018 commented Apr 12, 2024

Let the IOhandler successfully created by cmsOpenIOhandlerFromMem always has a valid Block.

@mm2
Copy link
Owner

mm2 commented Apr 12, 2024

Thanks for your contribución. Just to clarify the issue you are solving... how you do to get "Block" member set to NULL?

@x2018
Copy link
Contributor Author

x2018 commented Apr 12, 2024

Thank you for your prompt reply.

From the view of lcms, it is meaningless to set Block to NULL.
But for the users in practice, they may tranfer NULL due to unintentional reasons such as the memroy allocation problems.

After the changes, at least lcms will be more robust within its scope.

@mm2
Copy link
Owner

mm2 commented Apr 12, 2024

Sorry, I didn't explain myself. I mean, which sequence of lcms functions you call to get "Block" set to NULL?
I am asking because in the case it is not possible to set this value to NULL, then this check is useless, I.e, why not check if somebody has set it to 42 instead?

@x2018
Copy link
Contributor Author

x2018 commented Apr 13, 2024

I'm sorry that I misunderstood your meaning.

You are right, the first changed line of MemoryWrite() will indeed be theoretically useless, because the Block should not be NULL for a valid IOhandler after the second changed line takes effect.

Prior to this, as cmsOpenIOhandlerFromMem() itself is an exported function, the Block can be NULL if the Buffer passed to cmsOpenIOhandlerFromMem() is NULL in 'w' mode, since cmsOpenIOhandlerFromMem() only check it when it is 'r' mode without checking it in 'w' mode.

Because the test case crashed at memmove() in MemoryWrite(), I just made that change at the beginning.
Now I have removed it.

@mm2 mm2 merged commit 83f650b into mm2:master Apr 17, 2024
10 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.

2 participants