Skip to content

Commit 3575974

Browse files
author
Luca Toniolo
committed
inifile: Migrate C callers to new reentrant iniFindString() API
Follow-up to the C++ IniFile::Find() reentrancy fix. The previous commit changed Find() to return std::string by value, but the C API wrapper iniFind() still used static storage, making it non-reentrant. Add iniFindString() that takes a caller-provided buffer, making it safe for concurrent use and multiple sequential calls. Reimplement iniFind() as a deprecated wrapper around iniFindString(). Migrate all C callers to iniFindString(): - mb2hal: 11 call sites - vfdb_vfd: 2 call sites - vfs11_vfd: 4 call sites - halcmd: 2 call sites Also clean up dead commented-out code in mb2hal and halrmt.
1 parent e6ad67d commit 3575974

7 files changed

Lines changed: 51 additions & 65 deletions

File tree

‎src/hal/user_comps/mb2hal/mb2hal_init.c‎

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ retCode parse_common_section()
7979
char *fnct_name = "parse_common_section";
8080
char *section = "MB2HAL_INIT", *tag;
8181
const char *tmpstr;
82+
char tmpbuf[INI_MAX_LINELEN];
8283

8384
if (gbl.ini_file_ptr == NULL) {
8485
ERR(gbl.init_dbg, "gbl.ini_file_ptr NULL pointer");
@@ -90,7 +91,7 @@ retCode parse_common_section()
9091
DBG(gbl.init_dbg, "[%s] [%s] [%d]", section, tag, gbl.init_dbg);
9192

9293
tag = "VERSION"; //optional
93-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
94+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
9495
if (tmpstr != NULL) {
9596
int major, minor;
9697
sscanf(tmpstr, "%d.%d", &major, &minor);
@@ -99,7 +100,7 @@ retCode parse_common_section()
99100
DBG(gbl.init_dbg, "[%s] [%s] [%d]", section, tag, gbl.version);
100101

101102
tag = "HAL_MODULE_NAME"; //optional
102-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
103+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
103104
if (tmpstr != NULL) {
104105
gbl.hal_mod_name = strdup(tmpstr);
105106
}
@@ -176,6 +177,7 @@ retCode parse_transaction_section(const int mb_tx_num)
176177
char section[40];
177178
char *tag;
178179
const char *tmpstr;
180+
char tmpbuf[INI_MAX_LINELEN];
179181
mb_tx_t *this_mb_tx;
180182

181183
if (gbl.ini_file_ptr == NULL) {
@@ -197,7 +199,7 @@ retCode parse_transaction_section(const int mb_tx_num)
197199
snprintf(section, sizeof(section)-1, "TRANSACTION_%02d", mb_tx_num);
198200

199201
tag = "LINK_TYPE"; //required 1st time, then optional
200-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
202+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
201203
if (tmpstr != NULL) {
202204
if (strcasecmp(tmpstr, "tcp") == retOK) {
203205
this_mb_tx->cfg_link_type = linkTCP;
@@ -274,7 +276,7 @@ retCode parse_transaction_section(const int mb_tx_num)
274276

275277

276278
tag = "PIN_NAMES";
277-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
279+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
278280
if (tmpstr != NULL) {
279281
if(parse_pin_names(tmpstr, this_mb_tx) != retOK)
280282
{
@@ -344,7 +346,7 @@ retCode parse_transaction_section(const int mb_tx_num)
344346
DBG(gbl.init_dbg, "[%s] [%s] [%d]", section, tag, this_mb_tx->cfg_debug);
345347

346348
tag = "MB_TX_CODE"; //required
347-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
349+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
348350
if (tmpstr != NULL) {
349351
int i;
350352
for (i=0 ; i<mbtxMAX; i++) {
@@ -367,7 +369,7 @@ retCode parse_transaction_section(const int mb_tx_num)
367369
DBG(gbl.init_dbg, "[%s] [%s] [%s] [%d]", section, tag, this_mb_tx->mb_tx_fnct_name, this_mb_tx->mb_tx_fnct);
368370

369371
tag = "HAL_TX_NAME"; //optional
370-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
372+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
371373
if (tmpstr != NULL) {
372374
strncpy(this_mb_tx->hal_tx_name, tmpstr, HAL_NAME_LEN);
373375
}
@@ -376,32 +378,6 @@ retCode parse_transaction_section(const int mb_tx_num)
376378
}
377379
DBG(gbl.init_dbg, "[%s] [%s] [%s]", section, tag, this_mb_tx->hal_tx_name);
378380

379-
/*
380-
str = iniFind(gbl.ini_file_ptr, "PINNAME", mb_tx_name);
381-
if (str != NULL) {
382-
pin_name = malloc(strlen(str) + 1);
383-
rtapi_strxcpy(pin_name, str); // convert a const string into one
384-
// we can modify
385-
}
386-
else {
387-
pin_name = malloc(1); // empty string
388-
*pin_name = 0;
389-
}
390-
if (mb_tx->name[0] != 0) {
391-
strncpy(mb_tx_name, mb_tx->name, HAL_NAME_LEN);
392-
}
393-
else {
394-
snprintf(mb_tx_name, sizeof(mb_tx_name), "%02d", mb_tx_num);
395-
}
396-
memcpy(&gbl.mb_tx[mb_tx_num], mb_tx, sizeof(mb_tx_t));
397-
rc = create_pins(mb_tx_name, &gbl.mb_tx[mb_tx_num], pin_name);
398-
free(pin_name);
399-
if (rc != retOK) {
400-
ERR(gbl.init_dbg, "Failed to create pins");
401-
return retERR;
402-
}
403-
*/
404-
405381
return retOK;
406382
}
407383

@@ -410,6 +386,7 @@ retCode parse_tcp_subsection(const char *section, const int mb_tx_num)
410386
char *fnct_name="parse_tcp_subsection";
411387
char *tag;
412388
const char *tmpstr;
389+
char tmpbuf[INI_MAX_LINELEN];
413390
mb_tx_t *this_mb_tx;
414391

415392
if (gbl.ini_file_ptr == NULL || section == NULL) {
@@ -424,7 +401,7 @@ retCode parse_tcp_subsection(const char *section, const int mb_tx_num)
424401
this_mb_tx = &gbl.mb_tx[mb_tx_num];
425402

426403
tag = "TCP_IP"; //required 1st time, then optional
427-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
404+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
428405
if (tmpstr != NULL) {
429406
strncpy(this_mb_tx->cfg_tcp_ip, tmpstr, sizeof(this_mb_tx->cfg_tcp_ip)-1);
430407
}
@@ -466,6 +443,7 @@ retCode parse_serial_subsection(const char *section, const int mb_tx_num)
466443
char *fnct_name="parse_serial_subsection";
467444
char *tag;
468445
const char *tmpstr;
446+
char tmpbuf[INI_MAX_LINELEN];
469447
mb_tx_t *this_mb_tx;
470448

471449
if (gbl.ini_file_ptr == NULL || section == NULL) {
@@ -480,7 +458,7 @@ retCode parse_serial_subsection(const char *section, const int mb_tx_num)
480458
this_mb_tx = &gbl.mb_tx[mb_tx_num];
481459

482460
tag = "SERIAL_PORT"; //required 1st time
483-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
461+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
484462
if (tmpstr != NULL) {
485463
strncpy(this_mb_tx->cfg_serial_device, tmpstr, sizeof(this_mb_tx->cfg_serial_device)-1);
486464
}
@@ -548,7 +526,7 @@ retCode parse_serial_subsection(const char *section, const int mb_tx_num)
548526
DBG(gbl.init_dbg, "[%s] [%s] [%d]", section, tag, this_mb_tx->cfg_serial_data_bit);
549527

550528
tag = "SERIAL_PARITY"; //required 1st time
551-
tmpstr = iniFind(gbl.ini_file_ptr, tag, section);
529+
tmpstr = iniFindString(gbl.ini_file_ptr, tag, section, tmpbuf, sizeof(tmpbuf));
552530
if (tmpstr != NULL) {
553531
strncpy(this_mb_tx->cfg_serial_parity, tmpstr, sizeof(this_mb_tx->cfg_serial_parity)-1);
554532
}

‎src/hal/user_comps/vfdb_vfd/vfdb_vfd.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,12 @@ enum kwdresult {NAME_NOT_FOUND, KEYWORD_INVALID, KEYWORD_FOUND};
278278
int findkwd(param_pointer p, const char *name, int *result, const char *keyword, int value, ...)
279279
{
280280
const char *word;
281+
char wordbuf[INI_MAX_LINELEN];
281282
va_list ap;
282283
const char *kwds[MAX_KWD], **s;
283284
int nargs = 0;
284285

285-
if ((word = iniFind(p->fp, name, p->section)) == NULL)
286+
if ((word = iniFindString(p->fp, name, p->section, wordbuf, sizeof(wordbuf))) == NULL)
286287
return NAME_NOT_FOUND;
287288

288289
kwds[nargs++] = keyword;
@@ -311,6 +312,7 @@ int findkwd(param_pointer p, const char *name, int *result, const char *keyword,
311312
int read_ini(param_pointer p)
312313
{
313314
const char *s;
315+
char sbuf[INI_MAX_LINELEN];
314316
int value;
315317

316318
if ((p->fp = fopen(p->inifile,"r")) != NULL) {
@@ -328,7 +330,7 @@ int read_ini(param_pointer p)
328330
iniFindInt(p->fp, "MOTOR_HZ", p->section, &p->motor_hz);
329331
iniFindInt(p->fp, "MOTOR_RPM", p->section, &p->motor_rpm);
330332

331-
if ((s = iniFind(p->fp, "DEVICE", p->section))) {
333+
if ((s = iniFindString(p->fp, "DEVICE", p->section, sbuf, sizeof(sbuf)))) {
332334
p->device = strdup(s);
333335
}
334336
value = p->parity;

‎src/hal/user_comps/vfs11_vfd/vfs11_vfd.c‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -398,11 +398,12 @@ enum kwdresult {NAME_NOT_FOUND, KEYWORD_INVALID, KEYWORD_FOUND};
398398
int findkwd(param_pointer p, const char *name, int *result, const char *keyword, int value, ...)
399399
{
400400
const char *word;
401+
char wordbuf[INI_MAX_LINELEN];
401402
va_list ap;
402403
const char *kwds[MAX_KWD], **s;
403404
int nargs = 0;
404405

405-
if ((word = iniFind(p->fp, name, p->section)) == NULL)
406+
if ((word = iniFindString(p->fp, name, p->section, wordbuf, sizeof(wordbuf))) == NULL)
406407
return NAME_NOT_FOUND;
407408

408409
kwds[nargs++] = keyword;
@@ -431,6 +432,7 @@ int findkwd(param_pointer p, const char *name, int *result, const char *keyword,
431432
int read_ini(param_pointer p)
432433
{
433434
const char *s;
435+
char sbuf[INI_MAX_LINELEN];
434436
double f;
435437
int value;
436438

@@ -447,10 +449,10 @@ int read_ini(param_pointer p)
447449
iniFindInt(p->fp, "PORT", p->section, &p->tcp_portno);
448450
iniFindInt(p->fp, "RECONNECT_DELAY", p->section, &p->reconnect_delay);
449451

450-
if ((s = iniFind(p->fp, "TCPDEST", p->section))) {
452+
if ((s = iniFindString(p->fp, "TCPDEST", p->section, sbuf, sizeof(sbuf)))) {
451453
p->tcp_destip = strdup(s);
452454
}
453-
if ((s = iniFind(p->fp, "DEVICE", p->section))) {
455+
if ((s = iniFindString(p->fp, "DEVICE", p->section, sbuf, sizeof(sbuf)))) {
454456
p->device = strdup(s);
455457
}
456458
if (iniFindDouble(p->fp, "RESPONSE_TIMEOUT", p->section, &f)) {
@@ -478,10 +480,10 @@ int read_ini(param_pointer p)
478480
(void *)NULL) == KEYWORD_INVALID)
479481
return -1;
480482
#else
481-
if (iniFind(p->fp, "RTS_MODE", p->section) != NULL) {
483+
if (iniFindString(p->fp, "RTS_MODE", p->section, sbuf, sizeof(sbuf)) != NULL) {
482484
fprintf(stderr,"%s: warning - the RTS_MODE feature is not available with the installed libmodbus version (%s).\n"
483485
"to enable it, uninstall libmodbus-dev and rebuild with "
484-
"libmodbus built http://github.com/stephane/libmodbus:master .\n",
486+
"libmodbus built http://github.com/stephane/libmodbus:master .\n",
485487
LIBMODBUS_VERSION_STRING, p->progname);
486488
}
487489
#endif

‎src/hal/utils/halcmd.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
#include "emc/linuxcnc.h"
4747

4848
#ifndef NO_INI
49-
#include "inifile.h" /* iniFind() from libnml */
49+
#include "inifile.h" /* iniFindString() from libnml */
5050
FILE *halcmd_inifile = NULL;
5151
#endif
5252

@@ -707,7 +707,7 @@ static int replace_vars(char *source_str, char *dest_str, int max_chars, char **
707707
{
708708
int retval = 0;
709709
int next_delim, remaining, buf_space;
710-
char *replacement, sec[128], var[128];
710+
char *replacement, sec[128], var[128], ini_buf[INI_MAX_LINELEN];
711711
static char info[256];
712712
char *sp=source_str, *dp=dest_str, *secP, *varP;
713713
const char
@@ -776,11 +776,12 @@ static int replace_vars(char *source_str, char *dest_str, int max_chars, char **
776776
var[next_delim]='\0';
777777
if ( strlen(sec) > 0 ) {
778778
/* get value from INI file */
779-
/* cast to char ptr, we are discarding the 'const' */
780-
replacement = (char *) iniFind(halcmd_inifile, var, sec);
779+
replacement = (char *) iniFindString(halcmd_inifile, var, sec,
780+
ini_buf, sizeof(ini_buf));
781781
} else {
782782
/* no section specified */
783-
replacement = (char *) iniFind(halcmd_inifile, var, NULL);
783+
replacement = (char *) iniFindString(halcmd_inifile, var, NULL,
784+
ini_buf, sizeof(ini_buf));
784785
}
785786
if (replacement==NULL) {
786787
*detail = info;

‎src/hal/utils/halrmt.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,6 @@
303303
#include <rtapi_string.h> // rtapi_strlcpy
304304
#include "hal.h" // HAL public API decls
305305
#include "../hal_priv.h" // private HAL decls
306-
/* non-EMC related uses of halrmt may want to avoid libnml dependency
307-
#ifndef NO_INI
308-
#include "inifile.h" // iniFind() from libnml
309-
#endif
310-
#include <rtapi_string.h>
311-
*/
312306

313307
/***********************************************************************
314308
* LOCAL FUNCTION DECLARATIONS *

‎src/libnml/inifile/inifile.cc‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -592,19 +592,22 @@ IniFile::Exception::Print(FILE *fp)
592592

593593

594594
extern "C" const char *
595-
iniFind(FILE *fp, const char *tag, const char *section)
595+
iniFindString(FILE *fp, const char *tag, const char *section,
596+
char *buf, size_t bufsize)
596597
{
597-
IniFile f(false, fp);
598-
599-
// Note: Using static storage for C API backward compatibility.
600-
// This preserves the original non-reentrant behavior of the C API.
601-
// The returned pointer is only valid until the next call.
602-
// Callers needing to preserve the value should copy it immediately.
603-
static std::string result_storage;
604-
auto result = f.Find(tag, section);
598+
IniFile f(false, fp);
599+
auto result = f.FindString(buf, bufsize, tag, section);
605600
if (!result) return nullptr;
606-
result_storage = *result;
607-
return result_storage.c_str();
601+
return *result;
602+
}
603+
604+
extern "C" const char *
605+
iniFind(FILE *fp, const char *tag, const char *section)
606+
{
607+
// Deprecated: This function uses static storage and is not reentrant.
608+
// Use iniFindString() instead for thread-safe operation.
609+
static char result_storage[LINELEN];
610+
return iniFindString(fp, tag, section, result_storage, sizeof(result_storage));
608611
}
609612

610613
extern "C" int

‎src/libnml/inifile/inifile.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,13 @@ extern "C"
2525
{
2626
#endif
2727

28-
extern const char *iniFind(FILE *fp, const char *tag, const char *section);
28+
// Recommended buffer size for iniFindString()
29+
#define INI_MAX_LINELEN 256
30+
31+
extern const char *iniFind(FILE *fp, const char *tag, const char *section)
32+
__attribute__((deprecated("use iniFindString() instead")));
33+
extern const char *iniFindString(FILE *fp, const char *tag, const char *section,
34+
char *buf, size_t bufsize);
2935
extern int iniFindInt(FILE *fp, const char *tag, const char *section, int *result);
3036
extern int iniFindDouble(FILE *fp, const char *tag, const char *section, double *result);
3137
extern int TildeExpansion(const char *file, char *path, size_t size);

0 commit comments

Comments
 (0)