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

Question about the sentinel fuse. #88

Open
agnat opened this issue Jul 12, 2023 · 5 comments
Open

Question about the sentinel fuse. #88

agnat opened this issue Jul 12, 2023 · 5 comments

Comments

@agnat
Copy link

agnat commented Jul 12, 2023

First, let me thank you guys for making this. It's really fun to work with and it certainly beats appending a zip to the executable. 😁

I'm unsure about the sentinel though. On one hand the goal is compatibility with build-time injection using the linker. On the other hand, the only thing that prevents me from loading link-time resources is the sentinel fuse. I'd have to run sed or something...

Looking at the test.cpp it would seem it is safe to ignore postject_has_resource() since postject_find_resource(...)will return null if no resource is found. And indeed my application works better without postject_has_resource(). Now I can load both, resources injected at build-time and resources injected using postject.

So, my question is what is the sentinel/fuse actually guarding against? It just seems odd to introduce this artificial string substitution step since it breaks one of the main design goals: compatibility with link-time injection.

Unless the fuse has a hidden purpose that I don't understand just yet, I feel postject would be better of without it.

Thanks for looking into this...

@mhdawson
Copy link
Member

If it's what I think you are asking about the fuse is to ensure there is as close as possible to 0 impact on Node.js binaries shipped by the project. The furst ensures there is a close to zero overhead check so that when there is no bundled application there is no overhead.

@agnat
Copy link
Author

agnat commented Jul 12, 2023

Ah ok... a very low overhead pre-check makes sense. For build-time injection via linker-flags it would also make sense to set the fuse at build time. How about we somehow expose the initial value:

#ifndef POSTJECT_FUSE_VALUE
# define POSTJECT_FUSE_VALUE 0
#endif
// use stringify macros to build the actual string

... or with a bit more API:

#ifdef POSTJECT_INJECT_USING_LINKER  // TODO: better name?
# define POSTJECT_FUSE_VALUE ":1"
#else
# define POSTJECT_FUSE_VALUE ":0"
#endif 

Let me know which style you prefer and I make a PR...

@RaisinTen
Copy link
Contributor

RaisinTen commented Jul 14, 2023

FWIW, the initial value is already exposed as the POSTJECT_SENTINEL_FUSE macro in the header file:

postject/postject-api.h

Lines 20 to 23 in 3c4f208

#ifndef POSTJECT_SENTINEL_FUSE
#define POSTJECT_SENTINEL_FUSE \
"POSTJECT_SENTINEL_fce680ab2cc467b6e072b8b5df1996b2"
#endif

It can be customized at build-time if the POSTJECT_SENTINEL_FUSE macro is defined before the header is included. For reference, this is how it is done in Node.js: https://github.com/nodejs/node/blob/f9737b1b8c96e43cedfff764634524df62cf3109/src/node_sea.cc#L11-L18

// The POSTJECT_SENTINEL_FUSE macro is a string of random characters selected by
// the Node.js project that is present only once in the entire binary. It is
// used by the postject_has_resource() function to efficiently detect if a
// resource has been injected. See
// https://github.com/nodejs/postject/blob/35343439cac8c488f2596d7c4c1dddfec1fddcae/postject-api.h#L42-L45.
#define POSTJECT_SENTINEL_FUSE "NODE_SEA_FUSE_fce680ab2cc467b6e072b8b5df1996b2"
#include "postject-api.h"
#undef POSTJECT_SENTINEL_FUSE

Is this what you were looking for?

@agnat
Copy link
Author

agnat commented Jul 15, 2023

No. The macro POSTJECT_SENTINEL_FUSE defines the fuse name. I'd like to override the initial fuse value, which is currently a string literal. See https://github.com/nodejs/postject/blob/main/postject-api.h#L42

This is useful when embedding resources using the linker.

@RaisinTen
Copy link
Contributor

Ah, okay. I personally like the second option in #88 (comment) more than the first one. Feel free to send a PR. :)

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

No branches or pull requests

3 participants