mdsplus icon indicating copy to clipboard operation
mdsplus copied to clipboard

Conflicting declarations of SendArg (mdsipshr)

Open merlea opened this issue 3 years ago • 3 comments

In include/ipdesc.h SendArg is declared as:

extern int SendArg(int s, unsigned char i, char dtype, unsigned char nargs,
                   short len, char ndims, int *dims, char *ptr);

while in mdstcpip/mdsip_connections.h and mdstcpip/mdsipshr/SendArg.c it is defined and declared as:

int SendArg(int id, unsigned char idx, char dtype, unsigned char nargs,
            unsigned short length, char ndims, int *dims, char *bytes)

The difference is in the 5th argument length.

I think the declaration in include/ipdesc.h should be changed to unsigned short len. I am not sure whether other parts of the build would be impacted by this.

merlea avatar Oct 26 '21 17:10 merlea

I will see where else it is used and try to make it consistent

On 10/26/21 1:39 PM, Antoine Merle wrote:

In |include/ipdesc.h| |SendArg| is declared as:

|extern int SendArg(int s, unsigned char i, char dtype, unsigned char nargs, short len, char ndims, int *dims, char *ptr); |

while in |mdstcpip/mdsip_connections.h| and |mdstcpip/mdsipshr/SendArg.c| it is defined and declared as:

|int SendArg(int id, unsigned char idx, char dtype, unsigned char nargs, unsigned short length, char ndims, int *dims, char *bytes) |

The difference is in the 5th argument |length|.

I think the declaration in |include/ipdesc.h| should be changed to |unsigned short len|. I am not sure whether other parts of the build would be impacted by this.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MDSplus/mdsplus/issues/2404, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABY5AZLDD6EXOGMC5ANSOI3UI3YUZANCNFSM5GYL3KAA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Joshua Stillerman Research Engineer MIT Plasma Science and Fusion Center 617.253.8176 @.***

joshStillerman avatar Oct 26 '21 19:10 joshStillerman

Even though the 2nd declaration is more 'correct' the 1st one is in a public include and is used in many places inside of the MDSplus codebase.  I propose fixing / removing the definitions in the mdstcpip directory

Thoughts ?

On 10/26/21 1:39 PM, Antoine Merle wrote:

In |include/ipdesc.h| |SendArg| is declared as:

|extern int SendArg(int s, unsigned char i, char dtype, unsigned char nargs, short len, char ndims, int *dims, char *ptr); |

while in |mdstcpip/mdsip_connections.h| and |mdstcpip/mdsipshr/SendArg.c| it is defined and declared as:

|int SendArg(int id, unsigned char idx, char dtype, unsigned char nargs, unsigned short length, char ndims, int *dims, char *bytes) |

The difference is in the 5th argument |length|.

I think the declaration in |include/ipdesc.h| should be changed to |unsigned short len|. I am not sure whether other parts of the build would be impacted by this.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MDSplus/mdsplus/issues/2404, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABY5AZLDD6EXOGMC5ANSOI3UI3YUZANCNFSM5GYL3KAA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Joshua Stillerman Research Engineer MIT Plasma Science and Fusion Center 617.253.8176 @.***

joshStillerman avatar Oct 26 '21 19:10 joshStillerman

if you are saying that in one it is using signed whereas in the other it is using unsigned.. codewise this should not differ, before make quick fixes lets see when this was introduced.. and if it would really hurt to fix it to the better more correct version, which ever that may be, if the length refers to the descriptor length it has to be unsigned. but there was a reason why it is defined in two places, no?

zack-vii avatar Oct 26 '21 21:10 zack-vii

there are a bunch of places that call SendArg with 0 for this argument, will this cause a warning ? RemoteAccess.c appears to be the only place that calls it with signed.

joshStillerman avatar Mar 30 '23 19:03 joshStillerman