Login
Username:

Password:

Remember me



Lost Password?

Register now!

Sections

Who's Online
103 user(s) are online (57 user(s) are browsing Forums)

Members: 2
Guests: 101

trixie, saimo, more...

Headlines

 
  Register To Post  

Is Forbid() needed for SetMethod()?
Just can't stay away
Just can't stay away


See User information
Simple question for you gurus :)

When wrapping an existing interface function, do I need to use Forbid/Permit pair:

void newFunc() {
  
log(...);
  
oldFunc(...);
}

Forbid();
oldFunc SetMethod(newFunc);
Permit();


If so, why? I mean, if SetMethod requires Forbid(), why it doesn't handle it internally then?

EDIT: updated scenario a bit.


Edited by Capehill on 2019/4/9 15:58:39
Go to top
Re: Is Forbid() needed for SetMethod()?
Home away from home
Home away from home


See User information
@Capehill

I don't think it's needed, after all it only changes a pointer to another function, but I guess there is a danger of the library your changing might get flushed.
+ you might also increase the stack usage, of that function.

you might also run into issues where your patched function is executed in a forbid(), you might like to be extremely careful, not triggering any issue where a forbid() is run inside a forbid(). or a disable() inside a disable().



(NutsAboutAmiga)

Basilisk II for AmigaOS4
AmigaInputAnywhere
Excalibur
and other tools and apps.
Go to top
Re: Is Forbid() needed for SetMethod()?
Just popping in
Just popping in


See User information
Since SetMethod() replace SetFunction() I would assume that Forbid() and Permit() are required like Michael Sinz do it in SegTracker.c for example.

Go to top
Re: Is Forbid() needed for SetMethod()?
Quite a regular
Quite a regular


See User information
The AROS implementation of SetFunction does Forbid/Permit internally. The original developer docs also imply that you don't have to manually Forbid/Permit:
http://amigadev.elowar.com/read/ADCD_ ... cs_3._guide/node0238.html
As for SetMethod, I'd imagine that it also does whatever locking is needed for you. Still, there's no harm in doing Forbid/Permit around the block where you patch the functions, as you won't end up being rescheduled between the SetMethod calls.

This is just like television, only you can see much further.
Go to top
Re: Is Forbid() needed for SetMethod()?
Just popping in
Just popping in


See User information
@BSzili
Yep autodocs says that forbid/permit is for pre36 lib, so logically it doesn't need forbid/permit.
I wonder why then Mike Sinz do it anyway.

NOTE
SetFunction cannot be used on non-standard libraries like pre-V36
dos.library. Here you must manually Forbid(), preserve all 6
original bytes, set the new vector, SumLibrary(), then Permit().

Go to top
Re: Is Forbid() needed for SetMethod()?
Just can't stay away
Just can't stay away


See User information
Forbid() and permit() are evil, do not use them unless absolutly required and keep any code between them as short as possible.

If anyone remembers XStream the proposed AHI replacement, that was eventually abandoned due to the amount of forbid()/permit() calls being used system wide which caused XStream to stutter and click. Since then a lot of work has been done to remove them from OS4.

So basically you have to use them for OS3 but can usually avoid them for OS4 unless specificly told to use them eg. getting a list on currently open screens or windows where something opening or closing a screen or window in mid scan would cause havoc.

Amiga user since 1985
AOS4, A-EON, IBrowse & Alinea Betatester

Ps. I hate the new amigans website. <shudder>
Go to top
Re: Is Forbid() needed for SetMethod()?
Just popping in
Just popping in


See User information
@Kamelito

That dos.library note is just saying that dos.library <36 is "special" and is not set up normally due to it primarily being a way to call the internal functions supplied by Tripos.

I don't think that implies you do not need to Forbid()/Permit() when patching >=36 functions on any library.

Personally, I think it comes down to the library in question ... Some (like exec) would likely need a Disable if you change functions that can be called from interrupts, otherwise Forbid would be enough. You wouldn't want to change the function right as another program is calling it.

Go to top
Re: Is Forbid() needed for SetMethod()?
Just popping in
Just popping in


See User information
Nothing is written in the AmigaOS 4.x autodocs about forbid/permit like it was in CBM ones for SetMethod()
NOTE
THIS FUNCTION IS DANGEROUS. Don't use it. It's main purpose
is to provide ways for debuggers to hook into interfaces.

Go to top
Re: Is Forbid() needed for SetMethod()?
Not too shy to talk
Not too shy to talk


See User information
@all
I just checked the OS4 source and SetMethod contains a Forbid/Permit pair. So you are not required to set them yourself. And as Severin said:
Quote:
do not use them unless absolutly required


@LiveForIt
Quote:
I don't think it's needed, after all it only changes a pointer to another function

The pointer change itself is not necessarily an atomic operation and there's other stuff going on too, namely changing the interface's flags and the recalculation of the lib's checksum, so all in all some locking mechanism is needed. If it had to be the fat Forbid/Permit instead of a more lightweight mutex, don't know, I'm not deep enough into this to know about all implications here.

Go to top
Re: Is Forbid() needed for SetMethod()?
Just can't stay away
Just can't stay away


See User information
Forbid()/Permit() would be needed if the new function acts as a wrapper for the old library function instead of replacing it entirely.

int (*oldFunc)(void);

int newFunc(void) {
    
IExec->DebugPrintF("SomeFunc() called\n");
    return 
oldFunc();
}

/* ... */

    /* Patch SomeFunc() in ISomeLib with newFunc() */
    
IExec->Forbid();
    
oldFunc IExec->DoMethod((struct Interface *)ISomeLiboffsetof(struct SomeLibIFaceSomeFunc), newFunc);
    
IExec->Permit();


Without the Forbid() in the above example newFunc() might be called before oldFunc has been set, leading to an ISI as it tries to jump to a NULL pointer.

If the function in question is supposed to be callable from interrupts then Disable()/Enable() will need to be used instead of Forbid()/Permit().

Go to top
Re: Is Forbid() needed for SetMethod()?
Just can't stay away
Just can't stay away


See User information
Thanks for all comments. Example from salass00 is exactly my scenario so I think need to keep Forbid.

Go to top
Re: Is Forbid() needed for SetMethod()?
Home away from home
Home away from home


See User information
@salass00

That won't work anyway...

if you patch program is task1, it has all the memory and its function. task1 sets up patch, the patch is a really big complicated function that takes a long to execute maybe as much as 20 ms.

<exec switch to task 1>

task1 patches the function with new patch function.

<exec switch to task 2>

task2 start executing code from the patch function.

<exec switch to task 1>

task1 forbids exec to do scheduling.
task1 removed the patch function, and restores the old function.
task1 enable exec to do scheduling.
task1 quits, frees all memory.

<exec switch to task 3>

task3 allocates memory that privues was used by task1.

<exec switch to task 2>

task2 continue executing code from program counter it last executed code.

And now you got typical ISI or DSI error, we all love.

So forbid / permit does really help anything.

(NutsAboutAmiga)

Basilisk II for AmigaOS4
AmigaInputAnywhere
Excalibur
and other tools and apps.
Go to top
Re: Is Forbid() needed for SetMethod()?
Just can't stay away
Just can't stay away


See User information
@LiveForIt

The discussion was about installing a patch and not removal of one (as was my example code).

You are right though that removing an installed patch causes some problems in that after calling IExec->SetMethod() to restore the old function it is impossible to be 100% sure that there is not a program still using the patch function or just about to call it.

About the best you can do is add a large delay after and hope that it's enough.

Go to top

  Register To Post

 




Currently Active Users Viewing This Thread: 1 ( 0 members and 1 Anonymous Users )




Powered by XOOPS 2.0 © 2001-2023 The XOOPS Project