Hi Frank,
I'm not averse to any of these proposed changes. But before
making them, I would need to spend time making sure that they
worked and didn't cause any backward incompatibility with
applications, on as many platforms as I could. One of the key
goals in writing the code was portability, which is one reason
why the use of C features is conservative.
All of these suggestions can be fixed in a C++ specific
interface without potentially compromising the portability of
the C code which is why I suggested starting with that,
because it's less of a risk.
Ian
On 16/05/13 16:02, Frank Pagliughi wrote:
Ian,
Thanks for the quick reply.
On 05/16/2013 09:57 AM, Ian
Craggs wrote:
Hi Frank,
it's been my plan to have a C++
wrapper for the C APIs. If you are able to contribute
your code to this project, that would be great. If not,
I'll do my own. That's the place for proper C++
conformance.
Yes, I would be happy to contribute it. I can start posting
code within the next few weeks and would appreciate input on a
number of arbitrary design decisions.
(1) Make the headers C++ aware internally:
#if defined(__cplusplus)
extern "C" {
#endif
Once the C++ wrapper is written, then C++ programs can use
that, and this would become moot. I have a preference for
keeping C++ related stuff out of the C code, but I can see
the logic.
This one is pretty trivial, is fully C-conformant, and is
fairly common in C libraries these days. It just helps the C++
programmer in not having to remember which headers need to be
wrapped. It's not that you're adding any C++ to the header,
but rather just making it safe for a C++ app to use directly.
I think that a number of C++ apps might still use the C
library directly even if there were a C++ wrapper available.
(2) Observe proper const-ness for pointer parameters if
there is no intent in updating the item, particularly
"const char*" for "char*"
DLLExport int MQTTAsync_create(MQTTAsync*
handle,
const char* serverURI, const char* clientId, ...
When I've used const in the past, it's only caused pain.
Ah, yes, but the pain of not using it can be so much worse!
"The const and volatile properties are new with the ANSI
standard. The purpose of const is to announce objects that
may be placed in read-only memory, and perhaps to increase
opportunities for optimization."
Which is not the same as the purpose here. In the latest
version of gcc on my machine, using const seems not to cause
the problems I was expecting, but I'd have to check on all
the compilers we use, MS, and old versions of gcc, before
adding this change.
In C it is a good programming practice since it helps indicate
the direction of the parameter into or out of the function.
But in C++, it's a downright requirement. To use literal
strings you are forced by the compiler to cast away the
const-ness:
MQTTAsync_create(handle, (char*)
"tcp://localhost:1883", (char*) "MyClientID", ...
That's wrong, and pretty ugly too.
Just consider the C library these days. It uses const in this
way, like:
void *memcpy(void *dest, const void *src, size_t
n);
This indicates that the contents of 'dest' will be updated,
but 'src' will not.
Shouldn't the definition for clientId for example, be "const
char const * clientId"? That is, both the pointer and the
contents are immutable.
Well since the pointer itself it passed by value, you don't
need to do this for a function parameter, just like you
wouldn't bother to write "void do_something(const int n);"
But you would want to add the extra const if you were
declaring a constant string in memory:
Replace:
#define ADDRESS
"tcp://swtest.hursley.ibm.com:1883"
with:
const char* const ADDRESS =
"tcp://swtest.hursley.ibm.com:1883";
(3) Use const values instead of #define whenever possible,
since #define has no respect for namespaces, etc.
const int MQTTASYNC_SUCCESS = 0;
instead of
#define MQTTASYNC_SUCCESS 0
See above. I've encountered pain when this before when this
should be simple. This is better done in a C++ header I
think.
Well the Paho/MQTT macros are pretty-specifically named, so
probably won't cause a problem. But it must be remembered that
#define's will overwrite anything in other files that include
them regardless of language constraints, like namespaces. So
they can get ugly when an application mixes different 3rd
party libraries.
When pulling a C library into a C++ app, if the library uses
const declarations, any name clashes can be eliminated by
putting the C lib into a namespace of it's own:
// my_cpp_app.h
namespace some_c_lib {
#include "some_c_lib.h"
}
But #define's would "jump out" of that namespace.
BTW, I can submit a patch for most of this. I'm just now
starting to wander into MQTT and wanted to throw out the idea.
But I don't have a lot of different compilers to test it out
on at the moment
Thanks for your time,
Frank
|
