Skip to content

Fix: TCP setevent no longer throws bogus error message #2986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mwinkel-dev merged 6 commits into from
Nov 19, 2025
+69 −45
Merged

Fix: TCP setevent no longer throws bogus error message #2986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9874518
Fix: first TCP setevent no longer throws bogus error message
mwinkel-dev Nov 17, 2025
23e2581
Fix: rewrite setevent using INVALID_CONNECTION_ID
mwinkel-dev Nov 17, 2025
9366af2
Fix: the setevent utility now uses C exit codes
mwinkel-dev Nov 17, 2025
6721c2e
Fix: wfevent now uses C exit defines for clarity
mwinkel-dev Nov 17, 2025
1079327
Fix: wfevent now how consistent style on returns
mwinkel-dev Nov 17, 2025
8d7e819
Fix: rewrite sendRemoteEvent() so retries on each server if needed
mwinkel-dev Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 51 additions & 33 deletions mdsshr/MdsEvents.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ static void *handleRemoteAst(void *arg )
for (idx = 0; idx < num_receive_servers; idx++)
{
receive_thread_ids[idx] = searchOpenServer(receive_servers[idx]);
if(receive_thread_ids[idx] < 0)
if(receive_thread_ids[idx] <= INVALID_CONNECTION_ID)
Copy link
Contributor Author

@mwinkel-dev mwinkel-dev Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that around line 413 below, an error condition is detected whereupon a socket is set to zero (i.e., stdin) which seems a bit odd. The code works "as is", so it is probably OK to keep it.

{
receive_thread_ids[idx] = ConnectToMds_(receive_servers[idx]);
}
Expand All @@ -417,7 +417,7 @@ static void *handleRemoteAst(void *arg )
receive_thread_sockets[idx] = 0;
GetConnectionInfo_(receive_thread_ids[idx], 0, &receive_thread_sockets[idx], 0);
}
if (receive_thread_ids[idx] >= 0)
if (receive_thread_ids[idx] > INVALID_CONNECTION_ID)
{
status = MdsEventAst_(receive_thread_ids[idx], eventInfo->eventnam, eventInfo->astadr, eventInfo->astprm,
&curr_eventid);
Expand Down Expand Up @@ -456,7 +456,7 @@ static void *handleRemoteAst(void *arg )
for (i = 0; i < num_receive_servers; i++)
{

if (receive_thread_ids[i] > 0 && FD_ISSET(receive_thread_sockets[i], &readfds))
if (receive_thread_ids[i] > INVALID_CONNECTION_ID && FD_ISSET(receive_thread_sockets[i], &readfds))
{
m = GetMdsMsg_(receive_thread_ids[i], &status);
if (STATUS_OK &&
Expand Down Expand Up @@ -525,13 +525,13 @@ static void initializeRemote(int receive_events)
if (receive_events)
{
receive_ids[i] = searchOpenServer(servers[i]);
if (receive_ids[i] < 0)
if (receive_ids[i] <= INVALID_CONNECTION_ID)
receive_ids[i] = ConnectToMds_(servers[i]);
if (receive_ids[i] == INVALID_CONNECTION_ID)
{
printf("\nError connecting to %s\n", servers[i]);
perror("ConnectToMds_");
receive_ids[i] = 0;
receive_ids[i] = INVALID_CONNECTION_ID;
}
else
{
Expand All @@ -545,13 +545,13 @@ static void initializeRemote(int receive_events)
else
{
send_ids[i] = searchOpenServer(servers[i]);
if (send_ids[i] < 0)
if (send_ids[i] <= INVALID_CONNECTION_ID)
send_ids[i] = ConnectToMds_(servers[i]);
if (send_ids[i] == INVALID_CONNECTION_ID)
{
printf("\nError connecting to %s\n", servers[i]);
perror("ConnectToMds_");
send_ids[i] = 0;
send_ids[i] = INVALID_CONNECTION_ID;
}
else
{
Expand Down Expand Up @@ -884,14 +884,14 @@ static int canEventRemote(const int eventid)
KillHandler();
for (i = 0; i < num_receive_servers; i++)
{
if (receive_ids[i] < 0)
if (receive_ids[i] <= INVALID_CONNECTION_ID)
receive_ids[i] = ConnectToMds_(receive_servers[i]);
if(receive_ids[i] == INVALID_CONNECTION_ID)
{
printf("\nError connecting to %s\n", receive_servers[i]);
perror("ConnectToMds_");
}
if (receive_ids[i] > 0)
if (receive_ids[i] > INVALID_CONNECTION_ID)
status = MdsEventCan_(receive_ids[i], getRemoteId(eventid, i));
}
}
Expand All @@ -908,7 +908,11 @@ int RemoteMDSEventCan(const int eventid)
static int sendRemoteEvent(const char *const evname, const int data_len,
char *const data)
{
int status = 1, i, tmp_status;
int status, i, send_status, ans_status;
int tries;
const int MAX_TRIES = 3;
int bad_server = FALSE;
int bad_status;
char expression[256];
struct descrip ansarg;
struct descrip desc;
Expand All @@ -927,35 +931,49 @@ static int sendRemoteEvent(const char *const evname, const int data_len,
desc.dims[0] = data_len;
ansarg.ptr = 0;
sprintf(expression, "setevent(\"%s\"%s)", evname, data_len > 0 ? ",$" : "");
if (STATUS_OK)

for (i = 0; i < num_send_servers; i++)
{
int reconnects = 0;
tmp_status = 0;
for (i = 0; i < num_send_servers; i++)
{
if (send_ids[i] > 0)
for (tries = 0; tries < MAX_TRIES; tries++) {
send_status = MDSplusERROR;
ans_status = MDSplusERROR;

if (send_ids[i] > INVALID_CONNECTION_ID)
{
if (data_len > 0)
tmp_status =
MdsValue_(send_ids[i], expression, &desc, &ansarg, NULL);
else
tmp_status =
MdsValue_(send_ids[i], expression, &ansarg, NULL, NULL);
if (data_len > 0) {
send_status = MdsValue_(send_ids[i], expression, &desc, &ansarg, NULL);
} else {
send_status = MdsValue_(send_ids[i], expression, &ansarg, NULL, NULL);
}
if (IS_OK(send_status)) {
ans_status = (ansarg.ptr != NULL) ? *(int *)ansarg.ptr : MDSplusERROR;
}
}
if (tmp_status & 1)
tmp_status = (ansarg.ptr != NULL) ? *(int *)ansarg.ptr : 0;
if (!(tmp_status & 1))
{
status = tmp_status;
if (reconnects < 3)
{

if (IS_OK(send_status) && IS_OK(ans_status)) {
break;
} else {
ReconnectToServer(i, 0);
reconnects++;
i--;
}
}
free(ansarg.ptr);
}

if (tries >= MAX_TRIES) {
Copy link
Contributor Author

@mwinkel-dev mwinkel-dev Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: When a bad server is encountered, should the function continue sending the event to the remaining servers (as per this WIP)? Or should the function instead immediately terminate?

bad_server = TRUE;
if (IS_NOT_OK(send_status)) {
bad_status = send_status;
} else {
bad_status = ans_status;
}
}
free(ansarg.ptr);
}

// If one or more of the servers in the list cannot be reached, display the status of
// the last bad server. Otherwise, status is the answer from the last good server.
if (bad_server) {
status = bad_status;
} else {
status = ans_status;
}
return status;
}
Expand Down
8 changes: 7 additions & 1 deletion setevent/setevent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <stdio.h>
#include <string.h>
#include <status.h>
extern int MDSEvent(char *eventname, int len, char *data);

int main(int argc, char **argv)
Expand All @@ -32,12 +33,17 @@ int main(int argc, char **argv)
if (argc < 2)
{
printf("Usage: %s <event> [optional-data-string]\n", argv[0]);
status = 0;
status = C_OK;
}
else
{
int len = (int)((argc > 2) ? strlen(argv[2]) : 0);
status = MDSEvent(argv[1], len, argv[2]);
Copy link
Contributor Author

@mwinkel-dev mwinkel-dev Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the old code was returning an MDSplus status code (32-bits, 3 fields) when the function terminates. This is problematic because MDSplusSUCCESS = 65545, which when treated as a C exit code (a byte value) is a failure (i.e., echo $? displays 9 which is a non-zero value, thus denotes a C failure).

if (STATUS_OK) {
status = C_OK;
} else {
status = C_ERROR;
}
}
return (status);
}
22 changes: 11 additions & 11 deletions wfevent/wfevent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ int main(int argc, char **argv)
{
case '?':
printhelp(argv[0]);
return 0;
return C_OK;
break;
case 'd':
showdata = 1;
Expand All @@ -96,7 +96,7 @@ int main(int argc, char **argv)
break;
default:
printhelp(argv[0]);
return 1;
return C_ERROR;
Copy link
Contributor Author

@mwinkel-dev mwinkel-dev Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is a change in behavior. The define is C_ERROR = -1, which echo $? displays as 255.

The conjecture is that very few users have shell scripts that check the status code of wfevent. Thus, this change is likely very low risk.

Alternatively, we could add a new define, C_FAILURE = 1, and use it instead of C_ERROR.

break;
}
}
Expand All @@ -107,7 +107,7 @@ int main(int argc, char **argv)
else
{
printhelp(argv[0]);
return 1;
return C_ERROR;
}
}
}
Expand All @@ -124,7 +124,7 @@ int main(int argc, char **argv)
{
case 'h':
printhelp(argv[0]);
return 0;
return C_OK;
case 'd':
showdata = 1;
break;
Expand All @@ -137,15 +137,15 @@ int main(int argc, char **argv)
break;
default:
printhelp(argv[0]);
return 1;
return C_ERROR;
}
}
#endif
if (optind == argc)
{
printf("Missing event-name\n");
printhelp(argv[0]);
return 1;
return C_ERROR;
}
event = argv[optind];
status = MDSWfeventTimed(event, MAXDATA, data, &len, timeout);
Expand All @@ -166,13 +166,13 @@ int main(int argc, char **argv)
{
printf("Event %s occurred with data = %.*s\n", event, ans.length,
ans.pointer);
return 0;
return C_OK;
}
}
else
{
printf("Event %s occurred with invalid serialized data\n", event);
return 1;
return C_ERROR;
}
}
if (len > MAXDATA)
Expand Down Expand Up @@ -205,16 +205,16 @@ int main(int argc, char **argv)
{
printf("Event %s occurred.\n", event);
}
return (0);
return C_OK;
}
else if (timeout > 0)
{
printf("Event %s timed out.\n", event);
return 1;
return C_ERROR;
}
else
{
printf("Unknown error occurred\n");
return 1;
return C_ERROR;
}
}