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

PatchWork Reports InitSemaphore "Structure Is Not Cleared" #70

Closed
tygre-chingu opened this issue Jan 16, 2023 · 5 comments
Closed

PatchWork Reports InitSemaphore "Structure Is Not Cleared" #70

tygre-chingu opened this issue Jan 16, 2023 · 5 comments
Assignees

Comments

@tygre-chingu
Copy link

Hi!

I run PatchWork on my Amiga and it reports the issue "Severity 2: structure is not cleared" in relation to exec.library InitSemphore(). I think that this issue happens in SSL_read().

You can reproduce this issue by running PatchWork NOPERMIT MINOS=40 LEVEL=1 SMALL and, for example, IBrowse. In IBrowse, access a HTTP Web site (no "hits") and a HTTPS Web site (many "hits").

Thanks! 👍

@Futaura Futaura self-assigned this Jan 16, 2023
@Futaura Futaura added the bug label Jan 16, 2023
@Futaura
Copy link
Collaborator

Futaura commented Jan 16, 2023

How bizarre! I'm not sure I ever read that part of the autodoc - it is still there in OS4. Normally, the OS Init*() functions would initialize structures completely and not require the application to clear it themselves first. Usually, it is not a problem as InitSemaphore() is called on some global variable that would have been cleared by the OS at load time, but this won't apply to

#define ALLOCSEMAPHORE(sem, type) if ((sem = (type *)OPENSSL_malloc(sizeof(type))) != NULL) InitSemaphore((struct SignalSemaphore *)sem)
which is easily fixed, but I need to check the other calls to InitSemaphore() too

@tygre-chingu
Copy link
Author

Thanks Futaura :-)

@Futaura
Copy link
Collaborator

Futaura commented Jan 17, 2023

This was indeed only triggered by the one line of code mentioned earlier and even better it required just one character to be changed to fix it. I like these kind of fixes :-). For all the other instances, the semaphore memory will already be clear and I've tested the fix with PatchWork and those error reports are now gone. I was surprised to see how many times locks were being allocated - perhaps an optimisation to look into at some point.

It also only affected OS3 as the semaphore is instead allocated and initialised by AllocSysObjectTags() on OS4.

@Futaura Futaura closed this as completed Jan 17, 2023
@tygre-chingu
Copy link
Author

Wonderful, thanks! 👍

@tygre-chingu
Copy link
Author

Just to confirm that the "hits" disappeared with AmiSSL v5.7 :-)

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

No branches or pull requests

2 participants