Page MenuHomeFreeBSD

Introduce libcaspermock
ClosedPublic

Authored by oshogbo on Dec 11 2016, 11:50 PM.
Referenced Files
Unknown Object (File)
Dec 20 2023, 1:30 AM
Unknown Object (File)
Dec 13 2023, 8:17 PM
Unknown Object (File)
Nov 19 2023, 3:03 PM
Unknown Object (File)
Nov 14 2023, 7:39 PM
Unknown Object (File)
Nov 10 2023, 8:55 PM
Unknown Object (File)
Nov 10 2023, 7:09 PM
Unknown Object (File)
Nov 9 2023, 2:39 AM
Unknown Object (File)
Nov 7 2023, 8:34 PM
Subscribers

Details

Summary

Introduce libcaspermock. This library provides API exactly the same as libcasper. The different is that libcaspermock don't provide sandbox-friendly
functions. Adding this library allow us to simplify the code, we don't need check if libcasper is available in the system anymore, instead we just need to
use this library.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

oshogbo retitled this revision from to Introduce libcaspermock.
oshogbo updated this object.
oshogbo edited the test plan for this revision. (Show Details)
oshogbo added reviewers: cem, allanjude, ed, emaste, bapt.
oshogbo added a project: capsicum.

Why do we need a separate library for this? Why not just turn MK_CASPER=no into the equivalent of cap_enable() -> false?

The behavior of few functions are a little bit different libcasper and libcaspermock but this still could be merged somehow.
The only situation I can think of where you would like to have two libraries is when you would install something from ports which you don't want to use Casper and your base system is using Casper.

The only situation I can think of where you would like to have two libraries is when you would install something from ports which you don't want to use Casper and your base system is using Casper.

That feels a little contrived :-).

I don't really much like this approach, plus there is a high risk to have libcaspermock and libcasper out of sync

Maybe what I'm about to say is blasphemy in our circles, but it looks like this tries to solve a problem that an object oriented programming language with virtual functions (C++) could easily solve. libcasper's header file would provide declarations for abstract base classes for all sorts of handles. Then there are two implementations of these classes: one that acts as a no-op and one that is actually built on top of Capsicum. That way there is no need to resort to linker tricks.

Hi Guys :)

I changed the approach which should make most of you happy :)
With new implementation if we don't use Casper we don't need to link to any external library.
Most of the functions are implemented as macros or inline functions.

I know that this solution is not ideal, but this change block me a lot for further non-related to libcaspermock changes, so I hope we can proceed with that.
I don't thing also that this solutions don't close as any other solutions from implementation.

This approach was also approved and disused with pjd@.

Would you please upload a diff with full context (-U9999)? Thanks.

kdump/Makefile
17 ↗(On Diff #31884)

This change seems kind of gratuitous and makes this diff a lot bigger.

kdump/kdump.c
59 ↗(On Diff #31884)

Shouldn't all of these ifdefs go away with caspermock? I thought that was the whole point of having a mock casper?

kdump/Makefile
17 ↗(On Diff #31884)

Yes sorry, but we need to have the same as in the libcasper.h :(

kdump/kdump.c
59 ↗(On Diff #31884)

Yes it should but I want to simplify all usage of libcasper per application.
This is why I don't remove that yet and want to do it per separate review.

lib/libcasper/Makefile
3–4 ↗(On Diff #32131)

why is this needed?

lib/libcasper/Makefile.inc
5–7

what do you think about switching the sense and making it -DWITHOUT_CASPER or -DCASPER_MOCK or such?

oshogbo added inline comments.
lib/libcasper/Makefile
3–4 ↗(On Diff #32131)

Updated. Thanks.

lib/libcasper/Makefile.inc
5–7

The WITH_CASPER makes more sens because we are adding the shared libraries base on it. Like in ping case.

This revision is now accepted and ready to land.Sep 21 2017, 2:05 PM

As discussed with emaste@ I commited the changes regarding the stabilization of the ifdefs name in separate commit (r323866). This should make diff a little bit smaller.

This revision now requires review to proceed.Sep 21 2017, 2:44 PM

From in-person working group session, a suggestion to rename libcasper.h to libcasper.h.in and run unifdef on it during install.

% cat example.h.in
// example header file
#ifdef WITH_CASPER
// casper version
#else
// non-casper version
#endif

% unifdef -UWITH_CASPER -o example.h example.h.in
% cat example.h                                  
// example header file
// non-casper version

% unifdef -DWITH_CASPER -o example.h example.h.in
% cat example.h                                  
// example header file
// casper version

From in-person working group session, a suggestion to rename libcasper.h to libcasper.h.in and run unifdef on it during install.

% cat example.h.in
// example header file
#ifdef WITH_CASPER
// casper version
#else
// non-casper version
#endif

% unifdef -UWITH_CASPER -o example.h example.h.in
% cat example.h                                  
// example header file
// non-casper version

% unifdef -DWITH_CASPER -o example.h example.h.in
% cat example.h                                  
// example header file
// casper version

This works for the general build but has a few problems:

  • If a user changes from WITH_CASPER to WITHOUT_CASPER, the headers will not be rebuilt automatically. Doing so would require META_MODE to be enabled to rebuild when the command changes (-DWITHOUT_CASPER vs -DWITH_CASPER). Granted this problem sort of exists if we dont' use unifdef since all objects would need to be rebuilt if the command changes too.
  • The build silently passes regardless of WITH_CASPER or WITHOUT_CASPER or any discrepancy between what is being built and what may already be built. There's probably no way to solve this.
  • This is difficult to solve for the _rescue/_ build to disable casper in them since we need to generate new headers rather than just check in the headers for #if defined(RESCUE) || defined(WITHOUT_CASPER) at compile-time.

I know I'm rehashing an argument here but IMHO we don't gain much from all of this optional logic. Perhaps the reasoning for it needs to be explained here better. Why do we care about a MK_CASPER option? It does not look like a reasonable size argument from the sizes I'm seeing. The first implementation (https://reviews.freebsd.org/D8753?id=22822) of checking function cap_foo() { if (!cap_enable()) return (foo()); ... } seemed much more reasonable and easy to maintain. Perhaps the initial implementation could be improved with better feedback rather than dropping the approach entirely (the multi-lib issue was the biggest concern I saw and not the wrapper behavior). We can place any master override logic in cap_enable for people who don't want the complexity. If we really want these ifdefs I would rather just have something simple (not using unifdef) and just support #if defined(RESCUE) || defined(WITHOUT_CASPER) for the rare cases it is actually needed to be trimmed out and not bother with a default -DWITH_CASPER in the global CFLAGS...

This revision was automatically updated to reflect the committed changes.