Date   

Re: ​[PATCH 6.2.1] MAMA C++: Fixed memory leak in MamaMsgField

Damian Maguire <dmaguire@...>
 

Hey Victor,


In Github projects, forking is generally the best way to work with the project, and is the recommended approach for raising patches for OpenMAMA - you can actually see the forks used by the likes of myself, Frank and other contributors if you go looking.


Regarding some documentation, you can find a bunch in the 'Contributor Guides' section of the OpenMAMA GitHub page, with the 'Submission Process' page probably the most interesting for you: https://openmama.github.io/openmama_submission_process.html


If you still aren't comfortable with the approach once you've given it a try, or if you hit some difficulties, we can apply the patch for you, but I'd recommend you give it a go - it's a much nicer way of working with the project than sending patches around, and should let you use a bunch of GitHub's other functionality as well.


Any questions feel free to reach out to me.


Thanks,


Damian



DAMIAN MAGUIRE 

Senior Sales Engineer 

 

O. +44 289 568 0298  

M. +44 783 584 4770 

dmaguire@... 

 

Adelaide Exchange Building2nd Floor, 24-26 Adelaide StreetBelfast, BT2 8GD  

velatradingtech.com | @vela_tt





From: Victor Maleyev <imnotmindlin@...>
Sent: 08 November 2017 11:01
To: Frank Quinn
Cc: openmama-dev; Damian Maguire; frank@...
Subject: Re: [Openmama-dev] [PATCH 6.2.1] MAMA C++: Fixed memory leak in MamaMsgField
 
Hi Frank,

I don't know how to make pull requests without forking the project, that's why I sent the patch. Is it possible for you to apply it? Or is it any instruction on how to do pull requests?

08.11.2017, 02:41, "Frank Quinn" <fquinn.ni@...>:

Thanks Victor,

Yes I can see how that wouldn't have behaved as expected and fix looks sound. Are you going to raise a pull request against the "next" branch for this or do you want me to take it from here?

Cheers,
Frank


On Mon, 6 Nov 2017, 09:55 Victor Maleyev, <imnotmindlin@...> wrote:
Not sure if my previous mail reached the maillist. Adding list admins, sorry if this is inapropriate.
---
It is possible to reproduce memory leak using the following:

MamaMsgIterator i;
for(MamaMsgField field = msg.begin(i); ...; field =  *++i) {
    field.getVectorMsg(...);
}

Note that field is not reference but object.

When calling getVectorMsg() the array of pointers (mLastVectorMsg) is allocated by malloc but when we assign a new value to the field (field = *++i) the allocated array becomes unreachable and can't be deleted. Overloaded assignment operator calling clear before assignment can solve this.
Signed-off-by: Victor Maleyev <imnotmindlin@...>

From c036b0726464916b2e49d6b1297a2a82404c0359 Mon Sep 17 00:00:00 2001
From: Victor Maleyev <imnotmindlin@...>
Date: Fri, 3 Nov 2017 00:14:05 +0300
Subject: [PATCH] Fixed memory leak in MamaMsgField

Signed-off-by: Victor Maleyev <imnotmindlin@...>
---
 mama/c_cpp/src/cpp/MamaMsgField.cpp | 15 +++++++++++++++
 mama/c_cpp/src/cpp/mama/MamaMsgField.h | 6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/mama/c_cpp/src/cpp/MamaMsgField.cpp b/mama/c_cpp/src/cpp/MamaMsgField.cpp
index f2f287c..2cf1270 100644
--- a/mama/c_cpp/src/cpp/MamaMsgField.cpp
+++ b/mama/c_cpp/src/cpp/MamaMsgField.cpp
@@ -47,6 +47,21 @@ namespace Wombat
     {
     }

+ MamaMsgField::MamaMsgField (const MamaMsgField& field)
+ : mField (field.mField)
+ , mFieldDesc (NULL)
+ , mLastVectorMsg (NULL)
+ , mLastVectorMsgLen (0)
+ {
+ }
+
+ MamaMsgField& MamaMsgField::operator= (const MamaMsgField& field)
+ {
+ clear();
+ mField = field.mField;
+ return *this;
+ }
+
     void MamaMsgField::clear ()
     {
         if (mFieldDesc)
diff --git a/mama/c_cpp/src/cpp/mama/MamaMsgField.h b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
index b9b9e73..d8f45b1 100644
--- a/mama/c_cpp/src/cpp/mama/MamaMsgField.h
+++ b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
@@ -50,6 +50,12 @@ namespace Wombat
         MamaMsgField (
             mamaMsgField field);

+ MamaMsgField (
+ const MamaMsgField& field);
+
+ MamaMsgField& operator= (
+ const MamaMsgField& field);
+
         /**
          * Clear the field.
          */
--
2.13.3
-------- Конец пересылаемого сообщения --------_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev




The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC


Re: ​[PATCH 6.2.1] MAMA C++: Fixed memory leak in MamaMsgField

Victor Maleyev
 

Hi Frank,

I don't know how to make pull requests without forking the project, that's why I sent the patch. Is it possible for you to apply it? Or is it any instruction on how to do pull requests?

08.11.2017, 02:41, "Frank Quinn" <fquinn.ni@...>:

Thanks Victor,

Yes I can see how that wouldn't have behaved as expected and fix looks sound. Are you going to raise a pull request against the "next" branch for this or do you want me to take it from here?

Cheers,
Frank


On Mon, 6 Nov 2017, 09:55 Victor Maleyev, <imnotmindlin@...> wrote:
Not sure if my previous mail reached the maillist. Adding list admins, sorry if this is inapropriate.
---
It is possible to reproduce memory leak using the following:

MamaMsgIterator i;
for(MamaMsgField field = msg.begin(i); ...; field =  *++i) {
    field.getVectorMsg(...);
}

Note that field is not reference but object.

When calling getVectorMsg() the array of pointers (mLastVectorMsg) is allocated by malloc but when we assign a new value to the field (field = *++i) the allocated array becomes unreachable and can't be deleted. Overloaded assignment operator calling clear before assignment can solve this.
Signed-off-by: Victor Maleyev <imnotmindlin@...>

From c036b0726464916b2e49d6b1297a2a82404c0359 Mon Sep 17 00:00:00 2001
From: Victor Maleyev <imnotmindlin@...>
Date: Fri, 3 Nov 2017 00:14:05 +0300
Subject: [PATCH] Fixed memory leak in MamaMsgField

Signed-off-by: Victor Maleyev <imnotmindlin@...>
---
 mama/c_cpp/src/cpp/MamaMsgField.cpp | 15 +++++++++++++++
 mama/c_cpp/src/cpp/mama/MamaMsgField.h | 6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/mama/c_cpp/src/cpp/MamaMsgField.cpp b/mama/c_cpp/src/cpp/MamaMsgField.cpp
index f2f287c..2cf1270 100644
--- a/mama/c_cpp/src/cpp/MamaMsgField.cpp
+++ b/mama/c_cpp/src/cpp/MamaMsgField.cpp
@@ -47,6 +47,21 @@ namespace Wombat
     {
     }

+ MamaMsgField::MamaMsgField (const MamaMsgField& field)
+ : mField (field.mField)
+ , mFieldDesc (NULL)
+ , mLastVectorMsg (NULL)
+ , mLastVectorMsgLen (0)
+ {
+ }
+
+ MamaMsgField& MamaMsgField::operator= (const MamaMsgField& field)
+ {
+ clear();
+ mField = field.mField;
+ return *this;
+ }
+
     void MamaMsgField::clear ()
     {
         if (mFieldDesc)
diff --git a/mama/c_cpp/src/cpp/mama/MamaMsgField.h b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
index b9b9e73..d8f45b1 100644
--- a/mama/c_cpp/src/cpp/mama/MamaMsgField.h
+++ b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
@@ -50,6 +50,12 @@ namespace Wombat
         MamaMsgField (
             mamaMsgField field);

+ MamaMsgField (
+ const MamaMsgField& field);
+
+ MamaMsgField& operator= (
+ const MamaMsgField& field);
+
         /**
          * Clear the field.
          */
--
2.13.3
-------- Конец пересылаемого сообщения --------_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev




Re: ​[PATCH 6.2.1] MAMA C++: Fixed memory leak in MamaMsgField

Frank Quinn <fquinn.ni@...>
 

Thanks Victor,

Yes I can see how that wouldn't have behaved as expected and fix looks sound. Are you going to raise a pull request against the "next" branch for this or do you want me to take it from here?

Cheers,
Frank


On Mon, 6 Nov 2017, 09:55 Victor Maleyev, <imnotmindlin@...> wrote:
Not sure if my previous mail reached the maillist. Adding list admins, sorry if this is inapropriate.
---
It is possible to reproduce memory leak using the following:

MamaMsgIterator i;
for(MamaMsgField field = msg.begin(i); ...; field =  *++i) {
    field.getVectorMsg(...);
}

Note that field is not reference but object.

When calling getVectorMsg() the array of pointers (mLastVectorMsg) is allocated by malloc but when we assign a new value to the field (field = *++i) the allocated array becomes unreachable and can't be deleted. Overloaded assignment operator calling clear before assignment can solve this.
Signed-off-by: Victor Maleyev <imnotmindlin@...>

From c036b0726464916b2e49d6b1297a2a82404c0359 Mon Sep 17 00:00:00 2001
From: Victor Maleyev <imnotmindlin@...>
Date: Fri, 3 Nov 2017 00:14:05 +0300
Subject: [PATCH] Fixed memory leak in MamaMsgField

Signed-off-by: Victor Maleyev <imnotmindlin@...>
---
 mama/c_cpp/src/cpp/MamaMsgField.cpp | 15 +++++++++++++++
 mama/c_cpp/src/cpp/mama/MamaMsgField.h | 6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/mama/c_cpp/src/cpp/MamaMsgField.cpp b/mama/c_cpp/src/cpp/MamaMsgField.cpp
index f2f287c..2cf1270 100644
--- a/mama/c_cpp/src/cpp/MamaMsgField.cpp
+++ b/mama/c_cpp/src/cpp/MamaMsgField.cpp
@@ -47,6 +47,21 @@ namespace Wombat
     {
     }

+ MamaMsgField::MamaMsgField (const MamaMsgField& field)
+ : mField (field.mField)
+ , mFieldDesc (NULL)
+ , mLastVectorMsg (NULL)
+ , mLastVectorMsgLen (0)
+ {
+ }
+
+ MamaMsgField& MamaMsgField::operator= (const MamaMsgField& field)
+ {
+ clear();
+ mField = field.mField;
+ return *this;
+ }
+
     void MamaMsgField::clear ()
     {
         if (mFieldDesc)
diff --git a/mama/c_cpp/src/cpp/mama/MamaMsgField.h b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
index b9b9e73..d8f45b1 100644
--- a/mama/c_cpp/src/cpp/mama/MamaMsgField.h
+++ b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
@@ -50,6 +50,12 @@ namespace Wombat
         MamaMsgField (
             mamaMsgField field);

+ MamaMsgField (
+ const MamaMsgField& field);
+
+ MamaMsgField& operator= (
+ const MamaMsgField& field);
+
         /**
          * Clear the field.
          */
--
2.13.3
-------- Конец пересылаемого сообщения --------_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev


​[PATCH 6.2.1] MAMA C++: Fixed memory leak in MamaMsgField

Victor Maleyev
 

Not sure if my previous mail reached the maillist. Adding list admins, sorry if this is inapropriate.
---
It is possible to reproduce memory leak using the following:

MamaMsgIterator i;
for(MamaMsgField field = msg.begin(i); ...; field =  *++i) {
    field.getVectorMsg(...);
}

Note that field is not reference but object.

When calling getVectorMsg() the array of pointers (mLastVectorMsg) is allocated by malloc but when we assign a new value to the field (field = *++i) the allocated array becomes unreachable and can't be deleted. Overloaded assignment operator calling clear before assignment can solve this.
Signed-off-by: Victor Maleyev <imnotmindlin@yandex.ru>

From c036b0726464916b2e49d6b1297a2a82404c0359 Mon Sep 17 00:00:00 2001
From: Victor Maleyev <imnotmindlin@yandex.ru>
Date: Fri, 3 Nov 2017 00:14:05 +0300
Subject: [PATCH] Fixed memory leak in MamaMsgField

Signed-off-by: Victor Maleyev <imnotmindlin@yandex.ru>
---
 mama/c_cpp/src/cpp/MamaMsgField.cpp | 15 +++++++++++++++
 mama/c_cpp/src/cpp/mama/MamaMsgField.h | 6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/mama/c_cpp/src/cpp/MamaMsgField.cpp b/mama/c_cpp/src/cpp/MamaMsgField.cpp
index f2f287c..2cf1270 100644
--- a/mama/c_cpp/src/cpp/MamaMsgField.cpp
+++ b/mama/c_cpp/src/cpp/MamaMsgField.cpp
@@ -47,6 +47,21 @@ namespace Wombat
     {
     }

+ MamaMsgField::MamaMsgField (const MamaMsgField& field)
+ : mField (field.mField)
+ , mFieldDesc (NULL)
+ , mLastVectorMsg (NULL)
+ , mLastVectorMsgLen (0)
+ {
+ }
+
+ MamaMsgField& MamaMsgField::operator= (const MamaMsgField& field)
+ {
+ clear();
+ mField = field.mField;
+ return *this;
+ }
+
     void MamaMsgField::clear ()
     {
         if (mFieldDesc)
diff --git a/mama/c_cpp/src/cpp/mama/MamaMsgField.h b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
index b9b9e73..d8f45b1 100644
--- a/mama/c_cpp/src/cpp/mama/MamaMsgField.h
+++ b/mama/c_cpp/src/cpp/mama/MamaMsgField.h
@@ -50,6 +50,12 @@ namespace Wombat
         MamaMsgField (
             mamaMsgField field);

+ MamaMsgField (
+ const MamaMsgField& field);
+
+ MamaMsgField& operator= (
+ const MamaMsgField& field);
+
         /**
          * Clear the field.
          */
--
2.13.3
-------- Конец пересылаемого сообщения --------


Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux? [I]

Yury Batrakov
 

Classification: Public

Thanks for the details guys!

 

From: Frank Quinn [mailto:fquinn.ni@...]
Sent: Tuesday, October 03, 2017 8:53 PM
To: Yury Batrakov <yury.batrakov@...>
Cc: Damian Maguire <dmaguire@...>; openmama-users <openmama-users@...>; openmama-dev <openmama-dev@...>
Subject: Re: [Openmama-dev] Did MAMAIgnoreDeprecatedOpen ever work on Linux? [I]

 

Hi Yury,

 

Yes that method is deprecated because OpenMAMA prefers dynamic loading methods now (so that payloads don't need to be registered in OpenMAMA's core code to load or depend on magic characters).

 

Have you looked at mamaMsg_createForPayloadBridge? It should do the same thing, but accepts a payload bridge rather than a char identifier and is not deprecated.

 

Damian also provided some helper functions included 6.2.1 to help look up bridges by name which might be helpful for lookup and cache, including mama_getPayloadBridge.

 

Cheers,

Frank

 

On Tue, Oct 3, 2017 at 10:07 AM, Yury Batrakov <yury.batrakov@...> wrote:

Classification: For internal use only

Hey Damian,

 

OK, thank you for clarification. The reason why I thought that deprecation warnings should be deleted automatically (after defining MAMA_DLL and BRIDGE preprocessor variables) is the following declaration of mamaMsg_createForPayload

MAMAIgnoreDeprecatedOpen

MAMAExpDeprecatedDLL(

        "mamaMsg_createForPayload has been deprecated, use dynamic loading instead!")

extern mama_status

mamaMsg_createForPayload (mamaMsg* msg, const char id);

MAMAIgnoreDeprecatedClose

 

The background of this question is: I’m writing a middleware bridge which always has to create messages for specific payload - it’s a problem because mamaMsg_createForPayload is deprecated and mamaMsg_create creates messages for “default” payload - the payload initialized by a first bridge created by OpenMAMA. This will be a problem when two or more bridges are loaded to a process.

 

 

From: Damian Maguire [mailto:dmaguire@...]
Sent: Tuesday, October 03, 2017 10:27 AM
To: Yury Batrakov <yury.batrakov@...>; openmama-users <openmama-users@...>; openmama-dev <openmama-dev@...>
Subject: Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Hey Yury,

 

I'm not sure the behaviour you're describing is an issue - the purpose of the pragma's is to allow you to use 'deprecated' features of the API without generating warnings, rather than disable the deprecation completely. For example, when we implemented dynamic loading, we deprecated the 'mamaPayloadType' here. However, in order to support legacy clients, we continue to use the type within the MAMA codebase. We generally aim to be 'warning free' in the code, and understand the risks associated with using the type, so where we make use of mamaPayloadType we mark them with MamaIgnoreDeprecatedOpen and MamaIgnoreDeprecatedClose pragma's. This disables the deprecation specific warnings at those sites - for example, here.

 

Using your code, the equivalent would be:

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

MamaIgnoreDeprecatedOpen

        b();

MamaIgnoreDeprecatedClose

        return 0;

}

 

Which shouldn't warn about the use of b();.

 

Let me know if that makes sense, or if you have further questions around this.

 

Thanks,

 

Damian

 

 

DAMIAN MAGUIRE 

Senior Sales Engineer 

 

 

Adelaide Exchange Building, 2nd Floor, 24-26 Adelaide Street, Belfast, BT2 8GD  

velatradingtech.com | @vela_tt

 

 


From: openmama-dev-bounces@... <openmama-dev-bounces@...> on behalf of Yury Batrakov <yury.batrakov@...>
Sent: 02 October 2017 17:16
To: openmama-users; openmama-dev
Subject: [Openmama-dev] Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Classification: Public

Hi team,

 

Is MAMAIgnoreDeprecatedOpen supposed to work for Linux with gcc > 4.6?

See the following example:

 

// Next 3 lines were copied from wombat/…/linux/port.h

_Pragma ("GCC diagnostic push")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated\"")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

_Pragma ("GCC diagnostic pop")

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

        b();

        return 0;

}

 

When compiling with 4.8 it shows the following warnings anyway:

/opt/gcc/gcc-4.8.1/bin/g++ -Wall -Wextra 123.c

123.c: In function ‘int main()’:

123.c:15:9: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

         ^

123.c:15:11: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

           ^

 

But if we place those pragmas around invocation of b() (not around the definition) all warnings go away



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev

 



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux? [I]

Frank Quinn <fquinn.ni@...>
 

Hi Yury,

Yes that method is deprecated because OpenMAMA prefers dynamic loading methods now (so that payloads don't need to be registered in OpenMAMA's core code to load or depend on magic characters).

Have you looked at mamaMsg_createForPayloadBridge? It should do the same thing, but accepts a payload bridge rather than a char identifier and is not deprecated.

Damian also provided some helper functions included 6.2.1 to help look up bridges by name which might be helpful for lookup and cache, including mama_getPayloadBridge.

Cheers,
Frank

On Tue, Oct 3, 2017 at 10:07 AM, Yury Batrakov <yury.batrakov@...> wrote:

Classification: For internal use only

Hey Damian,

 

OK, thank you for clarification. The reason why I thought that deprecation warnings should be deleted automatically (after defining MAMA_DLL and BRIDGE preprocessor variables) is the following declaration of mamaMsg_createForPayload

MAMAIgnoreDeprecatedOpen

MAMAExpDeprecatedDLL(

        "mamaMsg_createForPayload has been deprecated, use dynamic loading instead!")

extern mama_status

mamaMsg_createForPayload (mamaMsg* msg, const char id);

MAMAIgnoreDeprecatedClose

 

The background of this question is: I’m writing a middleware bridge which always has to create messages for specific payload - it’s a problem because mamaMsg_createForPayload is deprecated and mamaMsg_create creates messages for “default” payload - the payload initialized by a first bridge created by OpenMAMA. This will be a problem when two or more bridges are loaded to a process.

 

 

From: Damian Maguire [mailto:dmaguire@...]
Sent: Tuesday, October 03, 2017 10:27 AM
To: Yury Batrakov <yury.batrakov@...>; openmama-users <openmama-users@lists.openmama.org>; openmama-dev <openmama-dev@....org>
Subject: Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Hey Yury,

 

I'm not sure the behaviour you're describing is an issue - the purpose of the pragma's is to allow you to use 'deprecated' features of the API without generating warnings, rather than disable the deprecation completely. For example, when we implemented dynamic loading, we deprecated the 'mamaPayloadType' here. However, in order to support legacy clients, we continue to use the type within the MAMA codebase. We generally aim to be 'warning free' in the code, and understand the risks associated with using the type, so where we make use of mamaPayloadType we mark them with MamaIgnoreDeprecatedOpen and MamaIgnoreDeprecatedClose pragma's. This disables the deprecation specific warnings at those sites - for example, here.

 

Using your code, the equivalent would be:

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

MamaIgnoreDeprecatedOpen

        b();

MamaIgnoreDeprecatedClose

        return 0;

}

 

Which shouldn't warn about the use of b();.

 

Let me know if that makes sense, or if you have further questions around this.

 

Thanks,

 

Damian

 

 

DAMIAN MAGUIRE 

Senior Sales Engineer 

 

 

Adelaide Exchange Building, 2nd Floor, 24-26 Adelaide Street, Belfast, BT2 8GD  

velatradingtech.com | @vela_tt

 

 


From: openmama-dev-bounces@lists.openmama.org <openmama-dev-bounces@lists.openmama.org> on behalf of Yury Batrakov <yury.batrakov@...>
Sent: 02 October 2017 17:16
To: openmama-users; openmama-dev
Subject: [Openmama-dev] Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Classification: Public

Hi team,

 

Is MAMAIgnoreDeprecatedOpen supposed to work for Linux with gcc > 4.6?

See the following example:

 

// Next 3 lines were copied from wombat/…/linux/port.h

_Pragma ("GCC diagnostic push")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated\"")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

_Pragma ("GCC diagnostic pop")

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

        b();

        return 0;

}

 

When compiling with 4.8 it shows the following warnings anyway:

/opt/gcc/gcc-4.8.1/bin/g++ -Wall -Wextra 123.c

123.c: In function ‘int main()’:

123.c:15:9: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

         ^

123.c:15:11: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

           ^

 

But if we place those pragmas around invocation of b() (not around the definition) all warnings go away



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.

_______________________________________________
Openmama-dev mailing list
Openmama-dev@....org
https://lists.openmama.org/mailman/listinfo/openmama-dev



Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux? [I]

Damian Maguire <dmaguire@...>
 

No problem Yury,


Yes, if I remember correctly the problem here was with some compilers which warned about the declaration of a deprecated method, which meant we wanted to both mark it as deprecated and also silence the deprecation warning at the same time. Not the most straight forward of things to manage in a portable cross platform fashion unfortunately.


Regarding the deprecated method, the 'mamaMsg_createForPayloadBridge ()' method was specifically created to handle this case - using the payload bridge itself to generate the message, rather than a simple identifier. There's an accessor for the payload bridges as well - 'mama_getPayloadBridge ()' (declared in mama.h) which you can use to search for an already loaded payload bridge. (it returns NULL in the case of a bridge not being found). Alternatively, 'mama_loadPayloadBridge ()' will either load the bridge or return an already loaded bridge if it exists.


Thanks,


Damian



DAMIAN MAGUIRE 

Senior Sales Engineer 

 

O. +44 289 568 0298  

M. +44 783 584 4770 

dmaguire@... 

 

Adelaide Exchange Building2nd Floor, 24-26 Adelaide StreetBelfast, BT2 8GD  

velatradingtech.com | @vela_tt





From: Yury Batrakov <yury.batrakov@...>
Sent: 03 October 2017 10:07
To: Damian Maguire; openmama-users; openmama-dev
Subject: RE: Did MAMAIgnoreDeprecatedOpen ever work on Linux? [I]
 

Classification: For internal use only

Hey Damian,

 

OK, thank you for clarification. The reason why I thought that deprecation warnings should be deleted automatically (after defining MAMA_DLL and BRIDGE preprocessor variables) is the following declaration of mamaMsg_createForPayload

MAMAIgnoreDeprecatedOpen

MAMAExpDeprecatedDLL(

        "mamaMsg_createForPayload has been deprecated, use dynamic loading instead!")

extern mama_status

mamaMsg_createForPayload (mamaMsg* msg, const char id);

MAMAIgnoreDeprecatedClose

 

The background of this question is: I’m writing a middleware bridge which always has to create messages for specific payload - it’s a problem because mamaMsg_createForPayload is deprecated and mamaMsg_create creates messages for “default” payload - the payload initialized by a first bridge created by OpenMAMA. This will be a problem when two or more bridges are loaded to a process.

 

 

From: Damian Maguire [mailto:dmaguire@...]
Sent: Tuesday, October 03, 2017 10:27 AM
To: Yury Batrakov <yury.batrakov@...>; openmama-users <openmama-users@...>; openmama-dev <openmama-dev@...>
Subject: Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Hey Yury,

 

I'm not sure the behaviour you're describing is an issue - the purpose of the pragma's is to allow you to use 'deprecated' features of the API without generating warnings, rather than disable the deprecation completely. For example, when we implemented dynamic loading, we deprecated the 'mamaPayloadType' here. However, in order to support legacy clients, we continue to use the type within the MAMA codebase. We generally aim to be 'warning free' in the code, and understand the risks associated with using the type, so where we make use of mamaPayloadType we mark them with MamaIgnoreDeprecatedOpen and MamaIgnoreDeprecatedClose pragma's. This disables the deprecation specific warnings at those sites - for example, here.

 

Using your code, the equivalent would be:

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

MamaIgnoreDeprecatedOpen

        b();

MamaIgnoreDeprecatedClose

        return 0;

}

 

Which shouldn't warn about the use of b();.

 

Let me know if that makes sense, or if you have further questions around this.

 

Thanks,

 

Damian

 

 

DAMIAN MAGUIRE 

Senior Sales Engineer 

 

O. +44 289 568 0298  

M. +44 783 584 4770 

 

Adelaide Exchange Building, 2nd Floor, 24-26 Adelaide Street, Belfast, BT2 8GD  

velatradingtech.com | @vela_tt

 

 


From: openmama-dev-bounces@... <openmama-dev-bounces@...> on behalf of Yury Batrakov <yury.batrakov@...>
Sent: 02 October 2017 17:16
To: openmama-users; openmama-dev
Subject: [Openmama-dev] Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Classification: Public

Hi team,

 

Is MAMAIgnoreDeprecatedOpen supposed to work for Linux with gcc > 4.6?

See the following example:

 

// Next 3 lines were copied from wombat/…/linux/port.h

_Pragma ("GCC diagnostic push")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated\"")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

_Pragma ("GCC diagnostic pop")

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

        b();

        return 0;

}

 

When compiling with 4.8 it shows the following warnings anyway:

/opt/gcc/gcc-4.8.1/bin/g++ -Wall -Wextra 123.c

123.c: In function ‘int main()’:

123.c:15:9: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

         ^

123.c:15:11: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

           ^

 

But if we place those pragmas around invocation of b() (not around the definition) all warnings go away



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.

The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC


Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux? [I]

Yury Batrakov
 

Classification: For internal use only

Hey Damian,

 

OK, thank you for clarification. The reason why I thought that deprecation warnings should be deleted automatically (after defining MAMA_DLL and BRIDGE preprocessor variables) is the following declaration of mamaMsg_createForPayload

MAMAIgnoreDeprecatedOpen

MAMAExpDeprecatedDLL(

        "mamaMsg_createForPayload has been deprecated, use dynamic loading instead!")

extern mama_status

mamaMsg_createForPayload (mamaMsg* msg, const char id);

MAMAIgnoreDeprecatedClose

 

The background of this question is: I’m writing a middleware bridge which always has to create messages for specific payload - it’s a problem because mamaMsg_createForPayload is deprecated and mamaMsg_create creates messages for “default” payload - the payload initialized by a first bridge created by OpenMAMA. This will be a problem when two or more bridges are loaded to a process.

 

 

From: Damian Maguire [mailto:dmaguire@...]
Sent: Tuesday, October 03, 2017 10:27 AM
To: Yury Batrakov <yury.batrakov@...>; openmama-users <openmama-users@...>; openmama-dev <openmama-dev@...>
Subject: Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Hey Yury,

 

I'm not sure the behaviour you're describing is an issue - the purpose of the pragma's is to allow you to use 'deprecated' features of the API without generating warnings, rather than disable the deprecation completely. For example, when we implemented dynamic loading, we deprecated the 'mamaPayloadType' here. However, in order to support legacy clients, we continue to use the type within the MAMA codebase. We generally aim to be 'warning free' in the code, and understand the risks associated with using the type, so where we make use of mamaPayloadType we mark them with MamaIgnoreDeprecatedOpen and MamaIgnoreDeprecatedClose pragma's. This disables the deprecation specific warnings at those sites - for example, here.

 

Using your code, the equivalent would be:

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

MamaIgnoreDeprecatedOpen

        b();

MamaIgnoreDeprecatedClose

        return 0;

}

 

Which shouldn't warn about the use of b();.

 

Let me know if that makes sense, or if you have further questions around this.

 

Thanks,

 

Damian

 

 

DAMIAN MAGUIRE 

Senior Sales Engineer 

 

O. +44 289 568 0298  

M. +44 783 584 4770 

 

Adelaide Exchange Building, 2nd Floor, 24-26 Adelaide Street, Belfast, BT2 8GD  

velatradingtech.com | @vela_tt

 

 


From: openmama-dev-bounces@... <openmama-dev-bounces@...> on behalf of Yury Batrakov <yury.batrakov@...>
Sent: 02 October 2017 17:16
To: openmama-users; openmama-dev
Subject: [Openmama-dev] Did MAMAIgnoreDeprecatedOpen ever work on Linux?

 

Classification: Public

Hi team,

 

Is MAMAIgnoreDeprecatedOpen supposed to work for Linux with gcc > 4.6?

See the following example:

 

// Next 3 lines were copied from wombat/…/linux/port.h

_Pragma ("GCC diagnostic push")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated\"")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

_Pragma ("GCC diagnostic pop")

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

        b();

        return 0;

}

 

When compiling with 4.8 it shows the following warnings anyway:

/opt/gcc/gcc-4.8.1/bin/g++ -Wall -Wextra 123.c

123.c: In function ‘int main()’:

123.c:15:9: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

         ^

123.c:15:11: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

           ^

 

But if we place those pragmas around invocation of b() (not around the definition) all warnings go away



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


Re: Did MAMAIgnoreDeprecatedOpen ever work on Linux?

Damian Maguire <dmaguire@...>
 

Hey Yury,


I'm not sure the behaviour you're describing is an issue - the purpose of the pragma's is to allow you to use 'deprecated' features of the API without generating warnings, rather than disable the deprecation completely. For example, when we implemented dynamic loading, we deprecated the 'mamaPayloadType' here. However, in order to support legacy clients, we continue to use the type within the MAMA codebase. We generally aim to be 'warning free' in the code, and understand the risks associated with using the type, so where we make use of mamaPayloadType we mark them with MamaIgnoreDeprecatedOpen and MamaIgnoreDeprecatedClose pragma's. This disables the deprecation specific warnings at those sites - for example, here.


Using your code, the equivalent would be:


int __attribute__((deprecated)) b() {

   return 0;

}

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

MamaIgnoreDeprecatedOpen

        b();

MamaIgnoreDeprecatedClose

        return 0;

}


Which shouldn't warn about the use of b();.


Let me know if that makes sense, or if you have further questions around this.


Thanks,


Damian



DAMIAN MAGUIRE 

Senior Sales Engineer 

 

O. +44 289 568 0298  

M. +44 783 584 4770 

dmaguire@... 

 

Adelaide Exchange Building2nd Floor, 24-26 Adelaide StreetBelfast, BT2 8GD  

velatradingtech.com | @vela_tt





From: openmama-dev-bounces@... <openmama-dev-bounces@...> on behalf of Yury Batrakov <yury.batrakov@...>
Sent: 02 October 2017 17:16
To: openmama-users; openmama-dev
Subject: [Openmama-dev] Did MAMAIgnoreDeprecatedOpen ever work on Linux?
 

Classification: Public

Hi team,

 

Is MAMAIgnoreDeprecatedOpen supposed to work for Linux with gcc > 4.6?

See the following example:

 

// Next 3 lines were copied from wombat/…/linux/port.h

_Pragma ("GCC diagnostic push")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated\"")

_Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

 

int __attribute__((deprecated)) b() {

   return 0;

}

 

_Pragma ("GCC diagnostic pop")

 

int main() {

        printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);

        b();

        return 0;

}

 

When compiling with 4.8 it shows the following warnings anyway:

/opt/gcc/gcc-4.8.1/bin/g++ -Wall -Wextra 123.c

123.c: In function ‘int main()’:

123.c:15:9: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

         ^

123.c:15:11: warning: ‘int b()’ is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]

         b();

           ^

 

But if we place those pragmas around invocation of b() (not around the definition) all warnings go away



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.

The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC


Did MAMAIgnoreDeprecatedOpen ever work on Linux?

Yury Batrakov
 

Classification: Public
Hi team,

Is MAMAIgnoreDeprecatedOpen supposed to work for Linux with gcc > 4.6?
See the following example:

// Next 3 lines were copied from wombat/.../linux/port.h
_Pragma ("GCC diagnostic push")
_Pragma ("GCC diagnostic ignored \"-Wdeprecated\"")
_Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

int __attribute__((deprecated)) b() {
return 0;
}

_Pragma ("GCC diagnostic pop")

int main() {
printf("GCC %d %d\n" , __GNUC__, __GNUC_MINOR__);
b();
return 0;
}

When compiling with 4.8 it shows the following warnings anyway:
/opt/gcc/gcc-4.8.1/bin/g++ -Wall -Wextra 123.c
123.c: In function 'int main()':
123.c:15:9: warning: 'int b()' is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]
b();
^
123.c:15:11: warning: 'int b()' is deprecated (declared at 123.c:7) [-Wdeprecated-declarations]
b();
^

But if we place those pragmas around invocation of b() (not around the definition) all warnings go away


---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


Re: OpenMAMA static analysis

Bill Torpey
 

Hi All:

I’ve completed a draft version of my latest article on static analysis, using clang and cppcheck on the OpenMAMA code.  It can be found at: http://btorpey.github.io/lots-o-static/  

I’m very interested in any comments, suggestions, etc. that the members of the community may have!

Regards,

Bill Torpey 

On Sep 19, 2017, at 1:10 PM, Duane Pauls <Duane.Pauls@...> wrote:

For Visual Studio, it appears as though the support lifecycle model changed after VS 2010.  While VS2012 and VS2013 are already EOL, VS2008 and VS2010 are still on extended support.  If the intent is to be cautious, it probably makes sense to wait until MS ends extended support before dropping support from OpenMAMA.
 
The end of extended support dates are:
VS2008: 2018-04-10
VS2010: 2020-07-14
 
Cheers,
Duane
 
From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Frank Quinn
Sent: Tuesday, September 19, 2017 11:17 AM
To: Bill Torpey
Cc: openmama-dev
Subject: Re: [Openmama-dev] OpenMAMA static analysis
 
The question of support for visual studio is one for the community which I'll actually pose on a separate thread with its own subject so that people might read it, but since VS2013 only went EOL from microsoft a year ago, I expect there are a few out there still using it. 
 
Last time we asked, there were still some 2008 users out there too. As you can see, we actually only got rid of 2005 support as recently as 2 years ago (I was keen to do it as 2008 introduced proper msbuild support so we could finally upgrade to newer visual studio project formats). I'm keen to move forward too, but I don't want to alienate our client base either, so any movement forward would need to have 100% community support.
 
Generally for static code analysis, I'm all for it - it tends to prevent issues that weren't even considered. However it would be a lower priority for me behind a few other outstanding CI tasks related to making us more comfortable in merging changes, namely:
  • CI and removal of warnings for Java builds
  • Unit tests in CI environment for C#
  • Windows CI for pull requests (possibly appveyor)
  • Set up of jenkins multi platform builds for windows and linux (which is now possible since they both use the same CI script)
It does continue the gradual increase in sophistication for CI we have had in the last couple of years though... first of all we got unit tests working, then we got warnings removed on linux, now they're gone on windows, but yes I definitely want to always be moving towards a better CI environment which includes static analysis... but they tend to be slow burning tasks so it might take a while to be part of our day-to-day maintenance, but I'd happily accept any static code analysis related changes in the interim.
 
Cheers,
Frank
 
On Tue, Sep 19, 2017 at 2:08 PM, Bill Torpey <wallstprog@...> wrote:
Hi Damian:  
 
Thanks for the feedback.  I’ve made some comments below.
 
Regards,
 
Bill
 
On Sep 19, 2017, at 3:17 AM, Damian Maguire <dmagdrums@...> wrote:
 
Hey Bill, 
 
Great to get your feedback here, and welcome to the dev list. Absolutely the right place for this sort of discussion - GitHub is I think are more geared towards individual issues rather than larger project discussions such as this. 
 
Myself and Frank have spent a good bit of time discussing the subject of static analysis in the OpenMAMA codebase actually (some of the other contributors I think are aware of my interest in that area), and I think the community as a whole would be more than happy to see a stronger focus on it going forward. When support was added for building with Clang, we actually added native support for the Clang static analysis tools via scan-build - https://github.com/OpenMAMA/OpenMAMA/blob/master/site_scons/site_tools/scan-build.py - which means you can actually build and analyse within a single step:
 
    scan-build --use-c=/path/to/clang --use-cxx=/path/to/clang++ --use-analyzer=/path/to/clang scons ...
 
I'd love to see similar integration with other tools going forward (including CPPCheck, and the Google Santizer Suite), so if you have experience in that area I'd be happy to review any contributions. 
 
Right now, getting static analysis working with other tools is somewhat dependent on the build tooling.  I use a custom build script that runs scons under bear to create the compilation database required by my scripts.  The build script is somewhat specific to my environment, so I don’t know how useful it would be to others. but I’ve attached it just in case.
 
 
 
However, once you’ve got a compilation database built (and cppcheck installed), you can use the scripts at https://github.com/btorpey/static to run analysis using both clang and cppcheck, e.g.:
 
cc_cppcheck.sh -i common/c_cpp/src/c/ -i mama/c_cpp/src/c/ -c 2>&1 | tee cppcheck.csv
 
Similar scripts exist for clang-check and clang-tidy (although afaict clang-tidy does everything that clang-check does and more).
 
 
Regarding any fixes you want to submit, I say get them raised and get them in - given Frank's Windows warning hunt is coming to an end (for now) tackling some of these feels like a natural next step. 
 
I guess I’m curious about what others think about relative priorities, and/or whether there is are natural “owners” for different parts of the code-base that might want to have input.
 


The only thing I would add, and related to your comment about the scope problems, is any changes will need to take into account the breadth of compilers OpenMAMA has to support and that unfortunately means continuing with the C89/C99 cross breed that older versions of Visual Studio rely on. This isn't so much an OpenMAMA stylistic choice as one that's been forced on us by compiler support, but the wider user community relies on us continuing to adhere to this as a standard (for now ;-)).  
 
Curious if there’s a timetable for requiring VS2013 (the first MS compiler that doesn’t have this problem).  As you’re probably aware, the K&R style is “officially” frowned on, at least in the C++ Core Guidelines (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es21-dont-introduce-a-variable-or-constant-before-you-need-to-use-it).  I notice that even some of Frank’s side projects (zmq, omnm) deviate from K&R style, which is understandable given how archaic it is.
 
In the meantime, it’s easy enough to filter out those warnings.
 
 
Great to hear you're getting to use OpenMAMA as part of your day job as well. Any feedback you have about your experience the community will be delighted to hear, including in the form of blog posts, so work away. If you want someone to review, or have any questions, I'm happy to help as well. 
 
Thanks.  I’ll send out a link when the article is complete — certainly interested in any feedback.  In the meantime, you may want to check out the other articles in the series: http://btorpey.github.io/blog/categories/static-analysis/
 
 
Looking forward to seeing more of your work. 
 
You’ve already seen some, just under another moniker: https://github.com/pulls?utf8=%E2%9C%93&q=author%3Abill-torpey-ullink+
 
 
 
 
 
 
 
 
Thanks, 
 
Damian 
 
On Sat, Sep 16, 2017 at 7:27 PM Bill Torpey <wallstprog@...> wrote:
Hi all:

I've been working with OpenMAMA for a little while now as part of my "day job", and I've also been working with static analysis tools for some time, so I thought it would be fun to put the two together.

As a first step, I ran the OpenMAMA code through cppcheck, which has become my go-to tool for static analysis, and produced the attached reports.  (FWIW, clang is good too, but I find that cppcheck flags more suspect code, and for me more is usually better).

I'm not sure what the rest of the community thinks about the importance of static analysis -- personally I'm a big believer, and I'm happy to get on board with helping to resolve some of these if others agree that would be a Good Thing.

For the record, the analysis was done on the 6.2.1 release, using cppcheck version 1.80, and using the scripts I've previously published as part of an ongoing series of articles about static analysis on [my blog](http://btorpey.github.io/).  The specific command used was:

   cc_cppcheck.sh 2>&1 | grep -v '/cpp' | grep -v test | grep -v examples | grep -v stdout | tee cppcheck-180.out

The idea was to concentrate on the core OpenMAMA C code, so the command explicitly ignores C++ code, test and examples.  (As well as some mysterious occurences of `stdout` which I haven't had time to track down yet).

A few observations:

- At least one of these issues has been [fixed subsequent to the release](https://github.com/OpenMAMA/OpenMAMA/pull/310).
- There are a boat-load of "scope can be reduced" warnings -- in most cases these can probably be explained by the K&R-style of declarations that OpenMAMA uses as a standard.  (Unfortunately, in my opinion ;-(
- Similarly, there are a bunch of "reassigned a value before the old one has been used", likely with the same cause.
- There are also a bunch of "argument ... names different" warnings -- at least one of these has [bitten me in the past](https://github.com/OpenMAMA/OpenMAMA/issues/297).

As mentioned above, I'm very interested in the community's thoughts on where to go from here (if anywhere).

Regards,

Bill Torpey

P.S.  I'm posting this to the mailing list, rather than to the GitHub issues, but please let me know if you think that would be a more appropriate place.

P.P.S.  I'm also writing another article in my series on static analysis in which I'll be using this analysis to discuss the benefits of static analysis in general, and cppcheck in particular.  This should in no way be seen as a criticism of the OpenMAMA code -- I've been working with OpenMama (as well as DataFabric) for quite a while, and it's a truly unique and impressive effort.  So, kudos to all for making OpenMAMA what it is -- but if we can make it even better, that's to everyone's benefit.

_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev
 


_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev



Re: RFC for Source Discovery

Keith Rudd
 

Classification: Public

My feedback:

 

Why was extending the existing MamaTransport Callback interface not considered as an approach?

 

All sources are available only in the context of a Mama Transport.

Middleware bridges already can use the Transport Callback interface to inform when the transport is connected, disconnected, has a data quality change etc.

It seems a fairly logical extension to me for basically the same mechanism (with an extra callback or two) to be used to relay other information about this connection (transport), including what sources are available on it, changes to the status of those, if that info becomes available.

 

I agree with the requirement to provide info on source names as strings, ids as integers.

For capabilities, using strings is perhaps best in practice to cope with the fact that the list of possible capabilities could change and may not be completely definable up front.

We need some standardization around this though (defined set of strings?) since I can see people wanting to process this capability info automatically and it is reasonable to expect that exactly equivalent capabilities could be provided by different middlewares – need to ensure they name them the same way in that case.

 

Regards,

Keith

 

From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Tom Doust
Sent: 20 September 2017 12:21
To: fquinn.ni@...; openmama-dev <openmama-dev@...>
Subject: Re: [Openmama-dev] RFC for Source Discovery

 

I have some feedback about the RFC for OpenMAMA Source Discovery page: https://openmama.github.io/openmama_rfc_source_discovery.html#

I’m pretty much in agreement with delivering the source descriptions and notification encoded in MAMA messages, it significantly reduces the work required to deliver an implementation and also simplifies things for application writers.

However, I’d like to suggest that rather than build a new API around a notional “source discovery object” as the RFC suggests, we simply make requests on the existing market data subscription API and differentiate by specifying a “special” source name (similar to how dictionary subscriptions are made ) This will work for both initial “discovered source” messages and updates to source status. This approach works well enough in the Tick42 TREP bridge. The main benefit of this approach is that in its simplest form it requires NO code to be written in the mama infrastructure and avoids having to replicate the API across each of the language bindings.

All the additional code devolves to the bridge implementation – this has to be written anyway. The special source name can be either fixed, hard coded in some way, or configurable through the mama.properties file. Although we would probably all agree as programmers it should be configurable, I can’t really think of a strong reason why J

I think that defining the fields as properties as indicated is necessary as it effectively defines the message schema but there are a couple of potential issues that are not covered by the RFC

  • Fid clashes. If the fids are defined as properties by the mama infrastructure there is a risk that they will overlap with fids defined in a dictionary posted by a bridge. This may not necessarily be a problem as the context may resolve any such clashes but nevertheless it might be worth exploring ways to avoid this
  • I think the fields need to be typed. Mostly they will be strings but some might be bool or int. This requires another property for each.

 

Is it possible to attribute a source with a data dictionary? I suspect that currently the assumptions made about field names in some of the MAMDA code effectively limit to a single data dictionary. This is an area that needs to be carefully explored and potentially tidied up.

Best regards

 

Tom Doust

 

TOM DOUST | Head of  Consultancy                                                                                             

TICK42

P: +44 (0) 1628 477444 | M: +44 (0) 7710 479924 | E: tom.doust@... | skype: tom.doust|  http://www.tick42.com

 

 

 

 

 

From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Frank Quinn
Sent: 13 September 2017 11:53
To: openmama-dev <openmama-dev@...>
Subject: [Openmama-dev] RFC for Source Discovery

 

Hi Folks,

 

We have a new RFC contributed by Arcontech to cover the upcoming Source Discovery functionality. Thanks very much to the Arcontech folks for taking the time to put this together.

 

You can find it listed here:

 

 

As a reminder of the RFC process and how to get involved, I'll refer you all to:

 

 

Pay particular attention to the most constructive ways to provide feedback: https://openmama.github.io/openmama_rfc_process.html#how-to-provide-rfc-feedback

 

As per our process, we will leave this RFC open for a period of 2 weeks, after which point if there are no major concerns, the RFC design will be considered approved and work can begin.

 

Cheers,

Frank



---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.


Code change(s) just landed on origin/next (Successful)

jenkins@...
 

Some changes have just been added to the origin/next branch!

[Frank Quinn] Added C# generated files to gitignore
	.gitignore

[Frank Quinn] Fixed build for C# unit tests
	mama/dotnet/src/nunittest/NUnitTest.csproj
	mama/dotnet/src/nunittest/MamaPublisherTest.cs
	mama/jni/src/junittests/MamaPublisherTest.java


Results for OpenMAMA_Snapshot_Linux CI run with latest changes:

  • CI Project Name: OpenMAMA_Snapshot_Linux
  • Build Number: #187
  • Build Status: Successful
  • Build Warnings: 0
  • Total Amount of Tests: 1829
  • Passed: 1829
  • Failed: 0
  • Skipped / Disabled: 0

You may also check CI console output to view the full results.


Re: RFC for Source Discovery

Tom Doust
 

I have some feedback about the RFC for OpenMAMA Source Discovery page: https://openmama.github.io/openmama_rfc_source_discovery.html#

I’m pretty much in agreement with delivering the source descriptions and notification encoded in MAMA messages, it significantly reduces the work required to deliver an implementation and also simplifies things for application writers.

However, I’d like to suggest that rather than build a new API around a notional “source discovery object” as the RFC suggests, we simply make requests on the existing market data subscription API and differentiate by specifying a “special” source name (similar to how dictionary subscriptions are made ) This will work for both initial “discovered source” messages and updates to source status. This approach works well enough in the Tick42 TREP bridge. The main benefit of this approach is that in its simplest form it requires NO code to be written in the mama infrastructure and avoids having to replicate the API across each of the language bindings.

All the additional code devolves to the bridge implementation – this has to be written anyway. The special source name can be either fixed, hard coded in some way, or configurable through the mama.properties file. Although we would probably all agree as programmers it should be configurable, I can’t really think of a strong reason why J

I think that defining the fields as properties as indicated is necessary as it effectively defines the message schema but there are a couple of potential issues that are not covered by the RFC

  • Fid clashes. If the fids are defined as properties by the mama infrastructure there is a risk that they will overlap with fids defined in a dictionary posted by a bridge. This may not necessarily be a problem as the context may resolve any such clashes but nevertheless it might be worth exploring ways to avoid this
  • I think the fields need to be typed. Mostly they will be strings but some might be bool or int. This requires another property for each.

 

Is it possible to attribute a source with a data dictionary? I suspect that currently the assumptions made about field names in some of the MAMDA code effectively limit to a single data dictionary. This is an area that needs to be carefully explored and potentially tidied up.

Best regards

 

Tom Doust

 

TOM DOUST | Head of  Consultancy                                                                                             

TICK42

P: +44 (0) 1628 477444 | M: +44 (0) 7710 479924 | E: tom.doust@... | skype: tom.doust|  http://www.tick42.com

 

 

 

 

 

From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Frank Quinn
Sent: 13 September 2017 11:53
To: openmama-dev <openmama-dev@...>
Subject: [Openmama-dev] RFC for Source Discovery

 

Hi Folks,

 

We have a new RFC contributed by Arcontech to cover the upcoming Source Discovery functionality. Thanks very much to the Arcontech folks for taking the time to put this together.

 

You can find it listed here:

 

 

As a reminder of the RFC process and how to get involved, I'll refer you all to:

 

 

Pay particular attention to the most constructive ways to provide feedback: https://openmama.github.io/openmama_rfc_process.html#how-to-provide-rfc-feedback

 

As per our process, we will leave this RFC open for a period of 2 weeks, after which point if there are no major concerns, the RFC design will be considered approved and work can begin.

 

Cheers,

Frank


Code change(s) just landed on origin/next (Successful)

jenkins@...
 

Some changes have just been added to the origin/next branch!

[noreply] Updated documentation for timedDispatch (#42)
	mama/c_cpp/src/c/mama/queue.h
	mama/dotnet/src/cs/MamaQueue.cs
	mama/c_cpp/src/cpp/mama/MamaQueue.h


Results for OpenMAMA_Snapshot_Linux CI run with latest changes:

  • CI Project Name: OpenMAMA_Snapshot_Linux
  • Build Number: #186
  • Build Status: Successful
  • Build Warnings: 0
  • Total Amount of Tests: 1829
  • Passed: 1829
  • Failed: 0
  • Skipped / Disabled: 0

You may also check CI console output to view the full results.


Code change(s) just landed on origin/next (Successful)

jenkins@...
 

Some changes have just been added to the origin/next branch!

[fquinn.ni] Fixed Unittest Failures (#323)
	mama/c_cpp/src/c/plugin.c


Results for OpenMAMA_Snapshot_Linux CI run with latest changes:

  • CI Project Name: OpenMAMA_Snapshot_Linux
  • Build Number: #185
  • Build Status: Successful
  • Build Warnings: 0
  • Total Amount of Tests: 1829
  • Passed: 1829
  • Failed: 0
  • Skipped / Disabled: 0

You may also check CI console output to view the full results.


Re: OpenMAMA static analysis

Duane Pauls
 

For Visual Studio, it appears as though the support lifecycle model changed after VS 2010.  While VS2012 and VS2013 are already EOL, VS2008 and VS2010 are still on extended support.  If the intent is to be cautious, it probably makes sense to wait until MS ends extended support before dropping support from OpenMAMA.

 

The end of extended support dates are:

VS2008: 2018-04-10

VS2010: 2020-07-14

 

Cheers,

Duane

 

From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Frank Quinn
Sent: Tuesday, September 19, 2017 11:17 AM
To: Bill Torpey
Cc: openmama-dev
Subject: Re: [Openmama-dev] OpenMAMA static analysis

 

The question of support for visual studio is one for the community which I'll actually pose on a separate thread with its own subject so that people might read it, but since VS2013 only went EOL from microsoft a year ago, I expect there are a few out there still using it.

 

Last time we asked, there were still some 2008 users out there too. As you can see, we actually only got rid of 2005 support as recently as 2 years ago (I was keen to do it as 2008 introduced proper msbuild support so we could finally upgrade to newer visual studio project formats). I'm keen to move forward too, but I don't want to alienate our client base either, so any movement forward would need to have 100% community support.

 

Generally for static code analysis, I'm all for it - it tends to prevent issues that weren't even considered. However it would be a lower priority for me behind a few other outstanding CI tasks related to making us more comfortable in merging changes, namely:

  • CI and removal of warnings for Java builds
  • Unit tests in CI environment for C#
  • Windows CI for pull requests (possibly appveyor)
  • Set up of jenkins multi platform builds for windows and linux (which is now possible since they both use the same CI script)

It does continue the gradual increase in sophistication for CI we have had in the last couple of years though... first of all we got unit tests working, then we got warnings removed on linux, now they're gone on windows, but yes I definitely want to always be moving towards a better CI environment which includes static analysis... but they tend to be slow burning tasks so it might take a while to be part of our day-to-day maintenance, but I'd happily accept any static code analysis related changes in the interim.

 

Cheers,

Frank

 

On Tue, Sep 19, 2017 at 2:08 PM, Bill Torpey <wallstprog@...> wrote:

Hi Damian: 

 

Thanks for the feedback.  I’ve made some comments below.

 

Regards,

 

Bill

 

On Sep 19, 2017, at 3:17 AM, Damian Maguire <dmagdrums@...> wrote:

 

Hey Bill, 

 

Great to get your feedback here, and welcome to the dev list. Absolutely the right place for this sort of discussion - GitHub is I think are more geared towards individual issues rather than larger project discussions such as this. 

 

Myself and Frank have spent a good bit of time discussing the subject of static analysis in the OpenMAMA codebase actually (some of the other contributors I think are aware of my interest in that area), and I think the community as a whole would be more than happy to see a stronger focus on it going forward. When support was added for building with Clang, we actually added native support for the Clang static analysis tools via scan-build - https://github.com/OpenMAMA/OpenMAMA/blob/master/site_scons/site_tools/scan-build.py - which means you can actually build and analyse within a single step:

 

    scan-build --use-c=/path/to/clang --use-cxx=/path/to/clang++ --use-analyzer=/path/to/clang scons ...

 

I'd love to see similar integration with other tools going forward (including CPPCheck, and the Google Santizer Suite), so if you have experience in that area I'd be happy to review any contributions. 

 

Right now, getting static analysis working with other tools is somewhat dependent on the build tooling.  I use a custom build script that runs scons under bear to create the compilation database required by my scripts.  The build script is somewhat specific to my environment, so I don’t know how useful it would be to others. but I’ve attached it just in case.

 

 

 

However, once you’ve got a compilation database built (and cppcheck installed), you can use the scripts at https://github.com/btorpey/static to run analysis using both clang and cppcheck, e.g.:

 

cc_cppcheck.sh -i common/c_cpp/src/c/ -i mama/c_cpp/src/c/ -c 2>&1 | tee cppcheck.csv

 

Similar scripts exist for clang-check and clang-tidy (although afaict clang-tidy does everything that clang-check does and more).

 

 

Regarding any fixes you want to submit, I say get them raised and get them in - given Frank's Windows warning hunt is coming to an end (for now) tackling some of these feels like a natural next step.

 

I guess I’m curious about what others think about relative priorities, and/or whether there is are natural “owners” for different parts of the code-base that might want to have input.

 



The only thing I would add, and related to your comment about the scope problems, is any changes will need to take into account the breadth of compilers OpenMAMA has to support and that unfortunately means continuing with the C89/C99 cross breed that older versions of Visual Studio rely on. This isn't so much an OpenMAMA stylistic choice as one that's been forced on us by compiler support, but the wider user community relies on us continuing to adhere to this as a standard (for now ;-)).  

 

Curious if there’s a timetable for requiring VS2013 (the first MS compiler that doesn’t have this problem).  As you’re probably aware, the K&R style is “officially” frowned on, at least in the C++ Core Guidelines (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es21-dont-introduce-a-variable-or-constant-before-you-need-to-use-it).  I notice that even some of Frank’s side projects (zmq, omnm) deviate from K&R style, which is understandable given how archaic it is.

 

In the meantime, it’s easy enough to filter out those warnings.

 

 

Great to hear you're getting to use OpenMAMA as part of your day job as well. Any feedback you have about your experience the community will be delighted to hear, including in the form of blog posts, so work away. If you want someone to review, or have any questions, I'm happy to help as well. 

 

Thanks.  I’ll send out a link when the article is complete — certainly interested in any feedback.  In the meantime, you may want to check out the other articles in the series: http://btorpey.github.io/blog/categories/static-analysis/

 

 

Looking forward to seeing more of your work. 

 

You’ve already seen some, just under another moniker: https://github.com/pulls?utf8=%E2%9C%93&q=author%3Abill-torpey-ullink+

 

 

 

 

 

 

 

 

Thanks, 

 

Damian 

 

On Sat, Sep 16, 2017 at 7:27 PM Bill Torpey <wallstprog@...> wrote:

Hi all:

I've been working with OpenMAMA for a little while now as part of my "day job", and I've also been working with static analysis tools for some time, so I thought it would be fun to put the two together.

As a first step, I ran the OpenMAMA code through cppcheck, which has become my go-to tool for static analysis, and produced the attached reports.  (FWIW, clang is good too, but I find that cppcheck flags more suspect code, and for me more is usually better).

I'm not sure what the rest of the community thinks about the importance of static analysis -- personally I'm a big believer, and I'm happy to get on board with helping to resolve some of these if others agree that would be a Good Thing.

For the record, the analysis was done on the 6.2.1 release, using cppcheck version 1.80, and using the scripts I've previously published as part of an ongoing series of articles about static analysis on [my blog](http://btorpey.github.io/).  The specific command used was:

   cc_cppcheck.sh 2>&1 | grep -v '/cpp' | grep -v test | grep -v examples | grep -v stdout | tee cppcheck-180.out

The idea was to concentrate on the core OpenMAMA C code, so the command explicitly ignores C++ code, test and examples.  (As well as some mysterious occurences of `stdout` which I haven't had time to track down yet).

A few observations:

- At least one of these issues has been [fixed subsequent to the release](https://github.com/OpenMAMA/OpenMAMA/pull/310).
- There are a boat-load of "scope can be reduced" warnings -- in most cases these can probably be explained by the K&R-style of declarations that OpenMAMA uses as a standard.  (Unfortunately, in my opinion ;-(
- Similarly, there are a bunch of "reassigned a value before the old one has been used", likely with the same cause.
- There are also a bunch of "argument ... names different" warnings -- at least one of these has [bitten me in the past](https://github.com/OpenMAMA/OpenMAMA/issues/297).

As mentioned above, I'm very interested in the community's thoughts on where to go from here (if anywhere).

Regards,

Bill Torpey

P.S.  I'm posting this to the mailing list, rather than to the GitHub issues, but please let me know if you think that would be a more appropriate place.

P.P.S.  I'm also writing another article in my series on static analysis in which I'll be using this analysis to discuss the benefits of static analysis in general, and cppcheck in particular.  This should in no way be seen as a criticism of the OpenMAMA code -- I've been working with OpenMama (as well as DataFabric) for quite a while, and it's a truly unique and impressive effort.  So, kudos to all for making OpenMAMA what it is -- but if we can make it even better, that's to everyone's benefit.

_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev

 


_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev

 


Re: OpenMAMA static analysis

Frank Quinn <fquinn.ni@...>
 

The question of support for visual studio is one for the community which I'll actually pose on a separate thread with its own subject so that people might read it, but since VS2013 only went EOL from microsoft a year ago, I expect there are a few out there still using it.

Last time we asked, there were still some 2008 users out there too. As you can see, we actually only got rid of 2005 support as recently as 2 years ago (I was keen to do it as 2008 introduced proper msbuild support so we could finally upgrade to newer visual studio project formats). I'm keen to move forward too, but I don't want to alienate our client base either, so any movement forward would need to have 100% community support.

Generally for static code analysis, I'm all for it - it tends to prevent issues that weren't even considered. However it would be a lower priority for me behind a few other outstanding CI tasks related to making us more comfortable in merging changes, namely:
  • CI and removal of warnings for Java builds
  • Unit tests in CI environment for C#
  • Windows CI for pull requests (possibly appveyor)
  • Set up of jenkins multi platform builds for windows and linux (which is now possible since they both use the same CI script)
It does continue the gradual increase in sophistication for CI we have had in the last couple of years though... first of all we got unit tests working, then we got warnings removed on linux, now they're gone on windows, but yes I definitely want to always be moving towards a better CI environment which includes static analysis... but they tend to be slow burning tasks so it might take a while to be part of our day-to-day maintenance, but I'd happily accept any static code analysis related changes in the interim.

Cheers,
Frank

On Tue, Sep 19, 2017 at 2:08 PM, Bill Torpey <wallstprog@...> wrote:
Hi Damian: 

Thanks for the feedback.  I’ve made some comments below.

Regards,

Bill

On Sep 19, 2017, at 3:17 AM, Damian Maguire <dmagdrums@...> wrote:

Hey Bill, 

Great to get your feedback here, and welcome to the dev list. Absolutely the right place for this sort of discussion - GitHub is I think are more geared towards individual issues rather than larger project discussions such as this. 

Myself and Frank have spent a good bit of time discussing the subject of static analysis in the OpenMAMA codebase actually (some of the other contributors I think are aware of my interest in that area), and I think the community as a whole would be more than happy to see a stronger focus on it going forward. When support was added for building with Clang, we actually added native support for the Clang static analysis tools via scan-build - https://github.com/OpenMAMA/OpenMAMA/blob/master/site_scons/site_tools/scan-build.py - which means you can actually build and analyse within a single step:

    scan-build --use-c=/path/to/clang --use-cxx=/path/to/clang++ --use-analyzer=/path/to/clang scons ...

I'd love to see similar integration with other tools going forward (including CPPCheck, and the Google Santizer Suite), so if you have experience in that area I'd be happy to review any contributions. 

Right now, getting static analysis working with other tools is somewhat dependent on the build tooling.  I use a custom build script that runs scons under bear to create the compilation database required by my scripts.  The build script is somewhat specific to my environment, so I don’t know how useful it would be to others. but I’ve attached it just in case.



However, once you’ve got a compilation database built (and cppcheck installed), you can use the scripts at https://github.com/btorpey/static to run analysis using both clang and cppcheck, e.g.:

cc_cppcheck.sh -i common/c_cpp/src/c/ -i mama/c_cpp/src/c/ -c 2>&1 | tee cppcheck.csv

Similar scripts exist for clang-check and clang-tidy (although afaict clang-tidy does everything that clang-check does and more).


Regarding any fixes you want to submit, I say get them raised and get them in - given Frank's Windows warning hunt is coming to an end (for now) tackling some of these feels like a natural next step.

I guess I’m curious about what others think about relative priorities, and/or whether there is are natural “owners” for different parts of the code-base that might want to have input.


The only thing I would add, and related to your comment about the scope problems, is any changes will need to take into account the breadth of compilers OpenMAMA has to support and that unfortunately means continuing with the C89/C99 cross breed that older versions of Visual Studio rely on. This isn't so much an OpenMAMA stylistic choice as one that's been forced on us by compiler support, but the wider user community relies on us continuing to adhere to this as a standard (for now ;-)).  

Curious if there’s a timetable for requiring VS2013 (the first MS compiler that doesn’t have this problem).  As you’re probably aware, the K&R style is “officially” frowned on, at least in the C++ Core Guidelines (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es21-dont-introduce-a-variable-or-constant-before-you-need-to-use-it).  I notice that even some of Frank’s side projects (zmq, omnm) deviate from K&R style, which is understandable given how archaic it is.

In the meantime, it’s easy enough to filter out those warnings.


Great to hear you're getting to use OpenMAMA as part of your day job as well. Any feedback you have about your experience the community will be delighted to hear, including in the form of blog posts, so work away. If you want someone to review, or have any questions, I'm happy to help as well. 

Thanks.  I’ll send out a link when the article is complete — certainly interested in any feedback.  In the meantime, you may want to check out the other articles in the series: http://btorpey.github.io/blog/categories/static-analysis/


Looking forward to seeing more of your work. 

You’ve already seen some, just under another moniker: https://github.com/pulls?utf8=%E2%9C%93&q=author%3Abill-torpey-ullink+








Thanks, 

Damian 

On Sat, Sep 16, 2017 at 7:27 PM Bill Torpey <wallstprog@...> wrote:
Hi all:

I've been working with OpenMAMA for a little while now as part of my "day job", and I've also been working with static analysis tools for some time, so I thought it would be fun to put the two together.

As a first step, I ran the OpenMAMA code through cppcheck, which has become my go-to tool for static analysis, and produced the attached reports.  (FWIW, clang is good too, but I find that cppcheck flags more suspect code, and for me more is usually better).

I'm not sure what the rest of the community thinks about the importance of static analysis -- personally I'm a big believer, and I'm happy to get on board with helping to resolve some of these if others agree that would be a Good Thing.

For the record, the analysis was done on the 6.2.1 release, using cppcheck version 1.80, and using the scripts I've previously published as part of an ongoing series of articles about static analysis on [my blog](http://btorpey.github.io/).  The specific command used was:

   cc_cppcheck.sh 2>&1 | grep -v '/cpp' | grep -v test | grep -v examples | grep -v stdout | tee cppcheck-180.out

The idea was to concentrate on the core OpenMAMA C code, so the command explicitly ignores C++ code, test and examples.  (As well as some mysterious occurences of `stdout` which I haven't had time to track down yet).

A few observations:

- At least one of these issues has been [fixed subsequent to the release](https://github.com/OpenMAMA/OpenMAMA/pull/310).
- There are a boat-load of "scope can be reduced" warnings -- in most cases these can probably be explained by the K&R-style of declarations that OpenMAMA uses as a standard.  (Unfortunately, in my opinion ;-(
- Similarly, there are a bunch of "reassigned a value before the old one has been used", likely with the same cause.
- There are also a bunch of "argument ... names different" warnings -- at least one of these has [bitten me in the past](https://github.com/OpenMAMA/OpenMAMA/issues/297).

As mentioned above, I'm very interested in the community's thoughts on where to go from here (if anywhere).

Regards,

Bill Torpey

P.S.  I'm posting this to the mailing list, rather than to the GitHub issues, but please let me know if you think that would be a more appropriate place.

P.P.S.  I'm also writing another article in my series on static analysis in which I'll be using this analysis to discuss the benefits of static analysis in general, and cppcheck in particular.  This should in no way be seen as a criticism of the OpenMAMA code -- I've been working with OpenMama (as well as DataFabric) for quite a while, and it's a truly unique and impressive effort.  So, kudos to all for making OpenMAMA what it is -- but if we can make it even better, that's to everyone's benefit.

_______________________________________________
Openmama-dev mailing list
Openmama-dev@....org
https://lists.openmama.org/mailman/listinfo/openmama-dev


_______________________________________________
Openmama-dev mailing list
Openmama-dev@....org
https://lists.openmama.org/mailman/listinfo/openmama-dev



Code change(s) just landed on origin/next (Successful)

jenkins@...
 

Some changes have just been added to the origin/next branch!

[Frank Quinn] Revert "MAMA: Moved background thread destroy to stop (#326)"
	mama/c_cpp/src/c/mama.c


Results for OpenMAMA_Snapshot_Linux CI run with latest changes:

  • CI Project Name: OpenMAMA_Snapshot_Linux
  • Build Number: #184
  • Build Status: Successful
  • Build Warnings: 0
  • Total Amount of Tests: 1829
  • Passed: 1829
  • Failed: 0
  • Skipped / Disabled: 0

You may also check CI console output to view the full results.


Re: Questions about MamaStartCallback

Frank Quinn <fquinn.ni@...>
 

Hi Yury,

I did have this proposed change merged, but have since discovered that it causes a deadlock in our unit test on windows.

On closer inspection, it looks like the problem is caused by the fact that making mama_stop blocking will effectively deadlock any application which attempts to call mama_stop from a MAMA event coming off the default queue (like our unit test application). Since I expect this is a common enough pattern, we'll have to revert this change.

Note that the argument to startBackground is misleadingly a stop callback - i.e. it will be called when mama_start *unblocks*. With this in mind, you should be able to get your MyMamaConnection destructor to wait on a semaphore raised by onStartComplete method (use mama_startBackgroundEx rather than mama_startBackground to provide a closure) to raise a semaphore to simulate the equivalent behaviour and prevent the crash.

Cheers,
Frank

On Mon, Aug 14, 2017 at 1:21 PM, Yury Batrakov <yury.batrakov@...> wrote:
Classification: Public

Hi guys,

As everybody knows there is possibility to start MAMA bridge in background thread in C++ API: function Mama::startBackground (mamaBridge bridgeImpl, MamaStartCallback* cb)
There are a couple of problems with this function though:
    1. We can't ignore "MAMA startup finished" event setting  the value for the second argument to nullptr - this can be fixed as a simple nullptr check in void MAMACALLTYPE stopCb (mama_status status, mamaBridge, void* closure) in mamacpp.cpp
    2. There is race condition between thread calling Mama::stop and internal MAMA thread spawned  by startBackground function. This race may lead to program crash.

Let me describe #2 - consider the following scenario:

We have a simple class MyMamaConnection

class MyMamaStartCallback : public MamaStartCallback {
    void onStartComplete (Wombat::MamaStatus status) override {
           // .... Some bridge specific callback code
    }
};

class MyMamaConnection {
    mamaBridge m_bridge;
    MyMamaStartCallback m_callback;

    MyMamaConnection() {
        Mama::startBackground (m_bridge, &m_callback)
    }

    ~MyMamaConnection() {
        Mama::stop(m_bridge);
    }
};

The crash occur in destructor because Mama::stop doesn't guarantee that no threads are still using m_callback pointer after this function exited. In our case MyMamaConnection's destructor destroys m_callback object while it is still in use by the thread created by Mama::startBackground.

Here are more details, file mama.c:

static void* mamaStartThread (void* closure)
{
...
    rval = mama_start (cb->mBridgeImpl); // Imagine user thread Thread_1 calling ~MyMamaConnection() from our example.

    // Current thread may be suspended by OS at this point while Thread_1 continues its work and deletes the object pointed by closure
...

    if (cb->mStopCallbackEx)

        // cb->mClosure points to destroyed object.

        cb->mStopCallbackEx (rval, cb->mBridgeImpl, cb->mClosure);
...
}

One of the possible solutions is to move this code from mama_close() to mama_stop():
                        // Get name of start background thread and destroy it
                        const char* threadName = wombatThread_getThreadName(middlewareLib->bridge->mStartBackgroundThread);
                        wombatThread_destroy (threadName);

Let me know if it's possible to have this fixed in the next release.


---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.
_______________________________________________
Openmama-dev mailing list
Openmama-dev@....org
https://lists.openmama.org/mailman/listinfo/openmama-dev

201 - 220 of 2305