close file descriptor

[matthewkeil]

This PR closes the leaked file descriptor in node.js bindings.

Relates to #246

[matthewkeil]

@asn-d6 I added the close check and matched the error message with the other similar errors in that function

[asn-d6]

LGTM! Feel free to merge!

[matthewkeil]

If fclose fails the setup is free'd where the error is thrown not in the cleanup function for clarity

[asn-d6]

@jtraglia i will wait for you to merge this one

[jtraglia]

One problem, the file descriptor is still leaked if the call to load_trusted_setup_file fails.

[jtraglia]

I believe that would work, but I would do it a little differently. I think this is simpler:

Napi::Value LoadTrustedSetup(const Napi::CallbackInfo& info) {
  Napi::Env env = info.Env();

  // Check if the trusted setup is already loaded
  KzgAddonData *data = env.GetInstanceData<KzgAddonData>();
  if (data->is_setup) {
    Napi::Error::New(env, "Error trusted setup is already loaded").ThrowAsJavaScriptException();
    return env.Undefined();
  }

  // Open the trusted setup file
  std::string file_path = info[0].As<Napi::String>().Utf8Value();
  FILE *file_handle = fopen(file_path.c_str(), "r");
  if (file_handle == nullptr) {
    Napi::Error::New(env, "Error opening trusted setup file: " + file_path).ThrowAsJavaScriptException();
    return env.Undefined();
  }

  // Load the trusted setup from that file
  C_KZG_RET ret = load_trusted_setup_file(&(data->settings), file_handle);

  // Close the trusted setup file
  if (fclose(file_handle) != 0) {
    if (ret == C_KZG_OK) {
      free_trusted_setup(&(data->settings));
    }
    Napi::Error::New(env, "Error closing trusted setup file").ThrowAsJavaScriptException();
    return env.Undefined();
  }

  // Check that it was successful
  if (ret != C_KZG_OK) {
    Napi::Error::New(env, "Error loading trusted setup file: " + file_path).ThrowAsJavaScriptException();
    return env.Undefined();
  }

  data->is_setup = true;
  return env.Undefined();
}

[matthewkeil]

I believe that would work, but I would do it a little differently. I think this is simpler:

You are a gentleman and a scholar sir!

[jtraglia]

LGTM 👍