Patching a library with SetFunction()
I've been working on a side project that deals with patching into a library (actually, a device, but the principles are the same) with SetFunction(). For inspiration, I've been looking at, amongst other things, the code in ISpy.c (http://amigadev.elowar.com/read/ADCD_2.1/AmigaMail_Vol2_guide/node018E…).
I understand that some kind of exclusion or lock is necessary when altering a library's function vectors, as you don't want another task to access the library which you're manipulating it. The code examples I've seen, such as ISpy, all seem to use a combination of Forbid()/Permit(), Disable()/Enable(), and/or semaphores.
I understand that this is done for maximum lockdown of the library during SetFunction(). The problem is, all the discussions in the various RKRM versions I've looked at actually discourage this type of programming. Indeed, they say Forbid() should never be used with Disable(), and that semaphores are an alternative, not a complement to these other exclusionary measures.
So, my question is: what is the proper way to set up exclusion around a SetFunction() operation? Is it enough with just Disable()/Enable() (which is what my reading of RKRM would imply)? Or is the "anti-pattern" of all three levels of lockdown the best way to proceed anyway?
Any advice from wiser minds is appreciated!
M.
An example of using Forbid()/Permit(), Disable()/Enable(), and semaphores with a device can be found in the source for HyperCache: https://github.com/PlummersSoftwareLLC/HyperCacheAmiga/blob/main/cache.c
This example only uses a combination of sempahores and Forbid()/Premit() to patch libraries with SetFunction(): https://github.com/Hagbard-Celin/galileo/blob/c12a4437f8665d980c9213251bd5bb1c27db3811/Source/Library/wb.c
LibraryInterceptor (https://aminet.net/package/dev/moni/liprgmsrc), on the other hand, only uses Forbid()/Permit() around its SetFunction(). Here's a snippet from LibraryInterceptor.c:
PRIVATE void SFReplace( void )
{
# ifdef DEBUG
fprintf( stderr, "NewFunction() == 0x%08LX\n", NewFunction );
# endif
Forbid();
OldLibraryVector = SetFunction( Exec_Base,
LVO_OpenLibrary,
NewFunction
);
// Clear the cpu's cache so the execution cache is valid:
CacheClearU();
Permit();
# ifdef DEBUG
fprintf( stderr, "OldLibraryVecotor == 0x%08LX\n", OldLibraryVector );
# endif
return;
}
Seeing all these different approaches is the cause of my wondering about best practice. In my case, I'm trying to make a debug routine for a device, i.e. patching the vector, outputting some diagnostic info, then passing the IORequest back to the original device function - pretty similar to what ISpy is doing with a library, and, indeed, what HyperCache does with a device.
Online Status
So Disable() is actually an even stronger lock down than Forbid()
When you Disable() you prevent interrupts and since interrupts creates taskswitches you effectively enter a forbid() too thus they should not be used together
You say you are doing a device, which to me is a warning sign that you deal with interrupts as well so I would choose Disable() if that is a concern though be quick
Now about semaphores they create critical regions. which in some ways are strong - in other ways less strong.
take your problem at hand
Without semaphore another task may be in the middle of executing some of the library code, when you do the disable()
But a semaphore would only solve that if it is your own library, as you would need to lock the semaphore in every potentially dangerous function.
I can't see why ispy would use semaphores except maybe to preven itself from setfunction in parallel. on the other hand the function install would now be called from different tasks, so that function would need to be careful about what it does. maybe that is where it uses a semaphore ?