OpenMAMA icon indicating copy to clipboard operation
OpenMAMA copied to clipboard

WIP: add more uuid_ functions

Open btorpey opened this issue 4 years ago • 12 comments

add more uuid_ functions

Summary

Some applications could use addl. uuid_ functions

Areas Affected

Place an 'x' within the braces to check the box

  • [x] MAMAC
  • [ ] MAMACPP
  • [ ] MAMADOTNET
  • [ ] MAMAJNI
  • [ ] MAMDA
  • [ ] MAMDACPP
  • [ ] MAMDADOTNET
  • [ ] MAMDAJNI
  • [ ] Visual Studio
  • [ ] SCons
  • [ ] Unit Tests
  • [ ] Examples

Details

Add more uuid_ functions -- in particular uuid_generate_times_safe which is useful for applications that must guarantee that uuid's are in fact unique.

Testing

Tested in separate application that uses OpenMAMA API.

btorpey avatar Mar 16 '20 15:03 btorpey

Note that Mac OS does not define uuid_generate_time_safe, but that's OK -- if the application cares it can do something like this:

   #ifdef wUuid_generate_time_safe
   if (wUuid_generate_time_safe(tempUuid) == -1) {
      return NULL;
   }
   #else
   if (wUuid_generate(tempUuid) == -1) {
      return NULL;
   }
   #endif

This provides a nice clean way for OpenMAMA to wrap system functions in a way that is portable. The alternative is to restrict OpenMAMA to a "lowest-common-denominator" approach, which arguably just creates more problems for applications.

bill-torpey avatar Mar 23 '20 20:03 bill-torpey

Not sure what to do about the Windows version -- that looks to be an existing issue (windows/wUuid.c):

void wUuid_generate_time (wUuid myUuid)
{
}

bill-torpey avatar Mar 23 '20 20:03 bill-torpey

Hi Bill is there any more work on this incoming here? If there's not a portable implementation for all platforms, I think this may be best served in external utility methods to maintain portability for OpenMAMA based applications.

fquinner avatar May 11 '20 19:05 fquinner

If you're suggesting getting rid of all the wUuuid stuff, I'm fine with that. That would also mean getting rid of the bogus implementation for Windows, I assume (windows/wUuuid.c):

void wUuid_generate_time (wUuid myUuid)
{
}

Then all of the uuid-related code becomes platform-dependent, and the responsibility of the application.

But as long as some of these functions are macro'd to wUuid... counterparts I don't see why adding more is a problem.

bill-torpey avatar May 11 '20 20:05 bill-torpey

No I'm saying if we add macros, they need to be available on all platforms and behave in the same way otherwise Openmama based apps may have portability issues. I.e. all available and behaving the same on Mac, Windows and Linux. I'm fine adding more (though wish we never started down that road), but they need to be consistent and portable.

fquinner avatar May 11 '20 20:05 fquinner

But that's not the current situation -- again, take a look at windows/wUuid.c and you'll see what I mean.

bill-torpey avatar May 11 '20 20:05 bill-torpey

But this PR doesn't contain any such noop or emulation functions for other platforms. Just Linux. And what you're referring to looks like a bug to me. If its just Linux then I don't understand why you would bother with the macros at all and not just use the non portable dereferenced alternative. Ideally though where no implementation exists, you would emulate one.

fquinner avatar May 11 '20 21:05 fquinner

Just trying to be a good citizen -- since wUuid... functions are already being #define'd, it seemed like the right thing to do was to stick with that convention.

In the process I noticed that the Windows implementation was a no-op, which made no sense, so I'm bringing it to your attention. (I can't really do Windows -- just Linux and Mac).

Short version, I think it should be all or nothing -- it doesn't make sense to me to #define a subset of wUuid... functions and not others. However, that's your call.

bill-torpey avatar May 11 '20 21:05 bill-torpey

Yeah and I appreciate that but that windows noop is a bug from the looks of it - certainly not the template from which to base new implementations.

These methods are portability methods - I can tell you from experience in days gone by that its no fun trying to port an openmama application and find out the common API methods you have used are not portable. If you can add Linux and Mac I can have a look at windows if you really want to see this PR through, but if its just for Linux, it wont be merged.

fquinner avatar May 11 '20 21:05 fquinner

Hi Frank:

OK, I see what you mean -- the 'wUuid_' functions that are defined are only the ones used internally by OpenMAMA, which are basically these:

./src/mama/c_cpp/src/c/bridge/qpid/transport.c:        wUuid_clear          (tempUuid);
./src/mama/c_cpp/src/c/bridge/qpid/transport.c:        wUuid_generate_time  (tempUuid);
./src/mama/c_cpp/src/c/bridge/qpid/transport.c:        wUuid_unparse        (tempUuid, uuidStringBuffer);
./src/mama/c_cpp/src/c/bridge/base/inbox.c:    wUuid_clear             (tempUuid);
./src/mama/c_cpp/src/c/bridge/base/inbox.c:    wUuid_generate_time     (tempUuid);
./src/mama/c_cpp/src/c/bridge/base/inbox.c:    wUuid_unparse           (tempUuid, uuidStringBuffer);

I was coming at it from a different point of view, which is to provide, as much as possible, OpenMAMA wrappers for the uuid_ functions that an OpenMAMA application might want to use.

For our purposes, the only addl. function that we care about is uuid_generate_time_safe -- and that is because we need to be absolutely certain that uuid's are in fact unique. Unfortunately, that function does not exist in Darwin, so the implementation for Mac would need to fall back to the arguably less safe uuid_generate_time (which OpenMAMA already uses).

I'm happy to try to provide an implementation for Linux & Mac that would use uuid_generate_time_safe if available and fallback to uuid_generate_time otherwise (leaving the Windows bit to you).

But I see your point about exposing (and then needing to support) addl. functions, so if you'd prefer to drop the whole thing I'm fine with that. If that's how you want to go, we'll just move that functionality into OZ.

Please let me know how you'd like to proceed.

Thanks!

bill-torpey avatar May 12 '20 14:05 bill-torpey

Yes if you want to take care of OSX and Linux, I can put together an interface compatible Windows version.

fquinner avatar Sep 16 '20 17:09 fquinner

Here's my take on mac impl -- note that wUuid_generate_time_safe is not #define'd, but simply implemented.

Turns out we need a way to know whether wUuid_generate_time_safe delegates, so we do something like this:

   #ifdef wUuid_generate_time_safe
   if (wUuid_generate_time_safe(tempUuid) == -1) {
      return NULL;
   }
   #else
   wUuid_generate(tempUuid);
   #endif

bill-torpey avatar Sep 18 '20 16:09 bill-torpey

This still something you're interested in Bill?

fquinner avatar Dec 05 '22 18:12 fquinner

In fact, yes. We use the following uuid functions in OZ:

#ifndef WUUID_H__
#define WUUID_H__

#include <uuid/uuid.h>

typedef uuid_t wUuid;

#define wUuid_generate              uuid_generate
#define wUuid_generate_time         uuid_generate_time
#define wUuid_generate_time_safe    uuid_generate_time_safe
#define wUuid_generate_random       uuid_generate_random

#define wUuid_unparse uuid_unparse

#define wUuid_clear(UUID) uuid_clear(UUID)

#endif /* WUUID_H__ */

bill-torpey avatar Dec 06 '22 18:12 bill-torpey

Cheers, taking this on now. Unfortunately it looks like windows UUID code never... actually generated UUIDs at all. The native API doesn't support this sort of depth of functionality either but will get as close as I can to the intended function based on what is available.

Windows does have Uuid generation support since Windows 2000, but getting it to work alongside these function signatures will be the challenge.

fquinner avatar Dec 07 '22 17:12 fquinner

Succeeded by #525 which includes windows support.

fquinner avatar Dec 08 '22 18:12 fquinner