context.Orion-LD icon indicating copy to clipboard operation
context.Orion-LD copied to clipboard

Dangling pointer introduced in #1329

Open rschwebel opened this issue 2 years ago • 8 comments

Unfortunately #1329 is locked, so I cannot comment directly in the change. I get a compile error:

/home/rsc/work/DistroKit/platform-v7a/build-target/orion-ld-2023-03-16-18d8e4fd5d8a88f795aac6942bb74b6aab4917b5/src/lib/orionld/mongoc/mongocEntityDelete.cpp: In function 'bool mongocEntityDelete(const char*, char**)':
/home/rsc/work/DistroKit/platform-v7a/build-target/orion-ld-2023-03-16-18d8e4fd5d8a88f795aac6942bb74b6aab4917b5/src/lib/orionld/mongoc/mongocEntityDelete.cpp:63:14: error: storing the address of local variable 'error' in '*detailP' [-Werror=dangling-pointer=]
   63 |     *detailP = error.message;
      |     ~~~~~~~~~^~~~~~~~~~~~~~~

This was introduced in 79380f41c2df2acfba1a46500a2bbf69622298a2 and indeed looks wrong. @kzangeli, could you have a look?

rschwebel avatar Mar 16 '23 20:03 rschwebel

Of course. I will have a look at it. Thanks for reporting!

kzangeli avatar Mar 16 '23 20:03 kzangeli

You probably meant

-    orionldError(OrionldInternalError, "Database Error", error.message, 500);
-    *detailP = error.message;
+    orionldError(OrionldInternalError, "Database Error", *detailP, 500);

Shall I send a patch?

rschwebel avatar Mar 16 '23 20:03 rschwebel

I need that string for the caller of the function. But of course, the char* can't point to a local variable and then used when the piece of stack "allocated" for that function has finished. Rookie mistake, had I been a rookie :)

I'm fixing it by allocating memory for "error.message" and return it that way. My compiler didn't catch this. Think I like your compiler better :) Thanks for reporting!

And BTW, your now stalled PR, funnily enough, about an hour ago, I started "cloning" your changes to merge the thing already :)

kzangeli avatar Mar 16 '23 21:03 kzangeli

Thanks for taking care! I noticed progress in this issue today, so I updated my test stack to the latest-and-greatest and found this issue...

rschwebel avatar Mar 16 '23 21:03 rschwebel

ok! I've fixed the bug and "hand-merged" the stuff from your PR into a branch that is now running my test suite. It'll end just around midnight but, I won't send the PR until tomorrow. A bit tired and about to go to bed! However, you'll have it all merged into develop by tomorrow, before lunch (assuming I don't run into any problem)

kzangeli avatar Mar 16 '23 21:03 kzangeli

This one: https://github.com/FIWARE/context.Orion-LD/pull/1273/commits/fe9ead85411dfde584c8d9d5eee3f8d808d28ce5 needs to be extended, as there are new defines. The rest of the series should still compile.

rschwebel avatar Mar 16 '23 21:03 rschwebel

Yeah, I did that. Looks like this now:

#define ORIONLD_URIPARAM_LANGUAGEPROPERTIES   (UINT64_C(1) << 32)
#define ORIONLD_URIPARAM_OBSERVEDAT           (UINT64_C(1) << 33)
#define ORIONLD_URIPARAM_LANG                 (UINT64_C(1) << 34)
#define ORIONLD_URIPARAM_LOCAL                (UINT64_C(1) << 35)
#define ORIONLD_URIPARAM_RESET                (UINT64_C(1) << 36)
#define ORIONLD_URIPARAM_LEVEL                (UINT64_C(1) << 37)

kzangeli avatar Mar 16 '23 21:03 kzangeli

Ack.

rschwebel avatar Mar 16 '23 21:03 rschwebel