modelica-3rdparty / msgpack-modelica

A MessagePack implementation as a Modelica package
BSD 2-Clause "Simplified" License
6 stars 4 forks source link

Rename msgpack-modelica.h to msgpack-modelica.c #2

Closed tbeu closed 10 years ago

tbeu commented 10 years ago

To make it possible to compile it stand-alone (as (shared) lib) msgpack-modelica.h should be renamed to msgpack-modelica.c. If it is a header file with implementation then it actually is a module file that should have c extension. Otherwise it looks like a dirty hack.

tbeu commented 10 years ago

In consequence Include dir is a Source dir.

sjoelund commented 10 years ago

For performance most of the functions are defined as static inline functions. These are removed from the C-file and will not be part of a library. They are also safe from multiple inclusions of the header in the same module. Most of the functions are simply there because in Modelica an external integer always has type "int" instead of size_t that msgpack uses. I prefer inline functions over macros because you get type safety and can compile with -O0 to debug the function calls easier.

Having the file as an Include also makes it possible to use the library without having to compile a library (something that is not standardised in Modelica). There are a few functions I would consider having as a part of a library. But they are so few compared to the bulk like msgpack_pack_int.

As a remark, this follows msgpack.h closely. Most of the simple msgpack.h are also inline functions in its header and are not a part of libmsgpackc.a. For example, you have:

static int msgpack_pack_int(msgpack_packer* pk, int d);

inline int msgpack_pack_int(msgpack_packer* x, int d)
{
# 424 "/usr/include/msgpack/pack_template.h" 3 4
 do { if(d < -(1<<5)) { if(d < -(1<<15)) { unsigned char buf[5]; buf[0] = 0xd2; do { uint32_t val = ntohl((int32_t)d); memcpy(&buf[1], &val, 4); } while(0)
; return (*(x)->callback)((x)->data, (const char*)buf, 5); } else if(d < -(1<<7)) { unsigned char buf[3]; buf[0] = 0xd1; do { uint16_t val = ntohs((int16_t
)d); memcpy(&buf[1], &val, 2); } while(0); return (*(x)->callback)((x)->data, (const char*)buf, 3); } else { unsigned char buf[2] = {0xd0, ((uint8_t*)&d)[0
]}; return (*(x)->callback)((x)->data, (const char*)buf, 2); } } else if(d < (1<<7)) { return (*(x)->callback)((x)->data, (const char*)&((uint8_t*)&d)[0], 
1); } else { if(d < (1<<8)) { unsigned char buf[2] = {0xcc, ((uint8_t*)&d)[0]}; return (*(x)->callback)((x)->data, (const char*)buf, 2); } else if(d < (1<<
16)) { unsigned char buf[3]; buf[0] = 0xcd; do { uint16_t val = ntohs((uint16_t)d); memcpy(&buf[1], &val, 2); } while(0); return (*(x)->callback)((x)->data
, (const char*)buf, 3); } else { unsigned char buf[5]; buf[0] = 0xce; do { uint32_t val = ntohl((uint32_t)d); memcpy(&buf[1], &val, 4); } while(0); return 
(*(x)->callback)((x)->data, (const char*)buf, 5); } } } while(0);
# 438 "/usr/include/msgpack/pack_template.h" 3 4
}
tbeu commented 10 years ago

static inline is nice for included compilation but not suitable for stand-alone compilation where I need to export symbols.

sjoelund commented 10 years ago

7f98ddb should contain the changes you need. With it you should be able to create a shared object and get an object with all symbols you would expect.