Skip to content

Commit

Permalink
Fix C headers including both ddog_Vec_U8 and ddog_prof_Vec_U8 (#83)
Browse files Browse the repository at this point in the history
**What does this PR do?**:

In #73, we changed a number of types, functions and enums to be
properly scoped inside or outside namespaces.

But while integrating the 1.0.1 release in Ruby, I noticed that by
mistake I forgot to properly configure the renaming for `Vec_U8`,
which resulted in two copies of `Vec_U8` being emitted in the header
files: `ddog_Vec_U8` and `ddof_prof_Vec_U8`, with some APIs using one
and others using the other.

This PR fixes the configuration.

**Motivation**:

Having two copies of the same struct is not correct and is really
annoying for clients as well -- I temporarily needed to duplicate
a helper on the Ruby profiler to have variants for both types
(since often we don't even use a pointer but the struct directly,
you can't even cast between them).

**Additional Notes**:

Here's the diff for the generated headers before/after this change:

```diff
diff -uwr old-include/datadog/common.h build-linux/x86_64-linux/include/datadog/common.h
--- old-include/datadog/common.h	2023-01-06 10:57:32.730644531 +0000
+++ build-linux/x86_64-linux/include/datadog/common.h	2023-01-06 10:58:41.954571430 +0000
@@ -106,16 +106,6 @@
  */
 typedef struct ddog_prof_Exporter_Request ddog_prof_Exporter_Request;

-/**
- * Holds the raw parts of a Rust Vec; it should only be created from Rust,
- * never from C.
- */
-typedef struct ddog_prof_Vec_U8 {
-  const uint8_t *ptr;
-  uintptr_t len;
-  uintptr_t capacity;
-} ddog_prof_Vec_U8;
-
 typedef enum ddog_prof_Exporter_NewResult_Tag {
   DDOG_PROF_EXPORTER_NEW_RESULT_OK,
   DDOG_PROF_EXPORTER_NEW_RESULT_ERR,
@@ -128,7 +118,7 @@
       struct ddog_prof_Exporter *ok;
     };
     struct {
-      struct ddog_prof_Vec_U8 err;
+      struct ddog_Vec_U8 err;
     };
   };
 } ddog_prof_Exporter_NewResult;
@@ -205,7 +195,7 @@
       struct ddog_HttpStatus http_response;
     };
     struct {
-      struct ddog_prof_Vec_U8 err;
+      struct ddog_Vec_U8 err;
     };
   };
 } ddog_prof_Exporter_SendResult;
@@ -399,7 +389,7 @@
 typedef struct ddog_prof_EncodedProfile {
   struct ddog_Timespec start;
   struct ddog_Timespec end;
-  struct ddog_prof_Vec_U8 buffer;
+  struct ddog_Vec_U8 buffer;
   struct ddog_prof_ProfiledEndpointsStats *endpoints_stats;
 } ddog_prof_EncodedProfile;

@@ -415,7 +405,7 @@
       struct ddog_prof_EncodedProfile ok;
     };
     struct {
-      struct ddog_prof_Vec_U8 err;
+      struct ddog_Vec_U8 err;
     };
   };
 } ddog_prof_Profile_SerializeResult;
diff -uwr old-include/datadog/profiling.h build-linux/x86_64-linux/include/datadog/profiling.h
--- old-include/datadog/profiling.h	2023-01-06 10:57:32.730644531 +0000
+++ build-linux/x86_64-linux/include/datadog/profiling.h	2023-01-06 10:58:41.954571430 +0000
@@ -224,7 +224,7 @@

 void ddog_prof_Profile_SerializeResult_drop(struct ddog_prof_Profile_SerializeResult _result);

-DDOG_CHECK_RETURN struct ddog_Slice_U8 ddog_Vec_U8_as_slice(const struct ddog_prof_Vec_U8 *vec);
+DDOG_CHECK_RETURN struct ddog_Slice_U8 ddog_Vec_U8_as_slice(const struct ddog_Vec_U8 *vec);

 /**
  * Resets all data in `profile` except the sample types and period. Returns
```

This is technically a breaking change for users of the C bindings,
since a type gets removed (even if the type was never supposed to
have existed).

Since #80 is already bumping the libdatadog version to 2.0.0,
I suggest only merging this PR after that one, and including it
in the 2.0.0 release.

**How to test the change?**:

This change doesn't actually affect the compiled library, only the
header files. Thus, the only way of noticing and testing this change,
is to compile something that includes the headers and validate that
only `ddog_Vec_U8` gets used.

Co-authored-by: Levi Morrison <levi.morrison@datadoghq.com>
  • Loading branch information
ivoanjo and morrisonlevi committed Jan 11, 2023
1 parent 2ed9132 commit 7f66aae
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions profiling-ffi/cbindgen.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ renaming_overrides_prefixing = true
"Tag" = "ddog_Tag"
"Timespec" = "ddog_Timespec"
"Vec_Tag" = "ddog_Vec_Tag"
"Vec_U8" = "ddog_Vec_U8"

"File" = "ddog_prof_Exporter_File"
"NewProfileExporterResult" = "ddog_prof_Exporter_NewResult"
Expand Down

0 comments on commit 7f66aae

Please sign in to comment.