Here is the patch adding re_extract_hash, against master-var-refactor.
I have added json_object_put, because without it it exhibits memory leak
in situation described below. Also I've added json_object_get somewhere.
Now, for me, memory is stable. But I must admit that it is severe
change, so it may lead to additional problems if variables can be passed
to varDelete without previous json_object_get.
--
Pavel Levshin
25.10.2013 19:16, Pavel Levshin:
25.10.2013 18:56, Rainer Gerhards:
I'll look at it.
But if you are going to allow such constructs:
set $!var = func( func_returning_object () )
then you need to free temporary object somewhere. Reference counting
is a
simplest way to do it.
well. json_object_object_get() is the only function that does not add
a new
reference, so there is no (kind of) temporary object created. As I
said, I
thought there's a bug and "fixed" it. It turned out that the fix does
double-frees, and so I went back to the doc...
OK, I see. But, in turn, there was no functions returning objects, as
far as I can see. If we introduce re_extract_all, this function will
create a temporary object and return it to the caller. If the caller
is, in turn, a function, then the object must be freed after use, just
like strings do. Where am I wrong?
ED by a myriad of sites beyond our control. PLEASE UNSUBSCRIBE and DO
NOT POST if you DON'T LIKE THAT.
---
grammar/rainerscript.c | 205 +++++++++++++++++++++++++++++++++++++-----------
grammar/rainerscript.h | 1 +
runtime/msg.c | 1 +
3 files changed, 162 insertions(+), 45 deletions(-)
diff --git a/grammar/rainerscript.c b/grammar/rainerscript.c
index f49d2aa..37f3509 100644
--- a/grammar/rainerscript.c
+++ b/grammar/rainerscript.c
@@ -1243,23 +1243,6 @@ var2CString(struct var *r, int *bMustFree)
return cstr;
}
-/* frees struct var members, but not the struct itself. This is because
- * it usually is allocated on the stack. Callers why dynamically allocate
- * struct var need to free the struct themselfes!
- */
-static void
-varFreeMembers(struct var *r)
-{
- /* Note: we do NOT need to free JSON objects, as we use
- * json_object_object_get() to obtain the values, which does not
- * increment the reference count. So json_object_put() [free] is
- * neither required nor permitted (would free the original object!).
- * So for the time being the string data type is the only one that
- * we currently need to free.
- */
- if(r->datatype == 'S') es_deleteStr(r->d.estr);
-}
-
static rsRetVal
doExtractFieldByChar(uchar *str, uchar delim, int matchnbr, uchar **resstr)
{
@@ -1351,12 +1334,14 @@ finalize_it:
RETiRet;
}
+#define RE_EXTRACT_MAX_PMATCH 50
+
static inline void
doFunc_re_extract(struct cnffunc *func, struct var *ret, void* usrptr)
{
size_t submatchnbr;
short matchnbr;
- regmatch_t pmatch[50];
+ regmatch_t pmatch[RE_EXTRACT_MAX_PMATCH];
int bMustFree;
es_str_t *estr;
char *str;
@@ -1377,7 +1362,7 @@ doFunc_re_extract(struct cnffunc *func, struct var *ret,
void* usrptr)
str = (char*) var2CString(&r[0], &bMustFree);
matchnbr = (short) var2Number(&r[2], NULL);
submatchnbr = (size_t) var2Number(&r[3], NULL);
- if(submatchnbr >= sizeof(pmatch)/sizeof(regmatch_t)) {
+ if(submatchnbr >= RE_EXTRACT_MAX_PMATCH) {
DBGPRINTF("re_extract() submatch %d is too large\n",
submatchnbr);
bHadNoMatch = 1;
goto finalize_it;
@@ -1427,9 +1412,9 @@ doFunc_re_extract(struct cnffunc *func, struct var *ret,
void* usrptr)
finalize_it:
if(bMustFree) free(str);
- varFreeMembers(&r[0]);
- varFreeMembers(&r[2]);
- varFreeMembers(&r[3]);
+ varDelete(&r[0]);
+ varDelete(&r[2]);
+ varDelete(&r[3]);
if(bHadNoMatch) {
cnfexprEval(func->expr[4], &r[4], usrptr);
@@ -1444,6 +1429,110 @@ finalize_it:
return;
}
+static inline void
+doFunc_re_extract_hash(struct cnffunc *func, struct var *ret, void* usrptr)
+{
+ short matchnbr;
+ regmatch_t pmatch[RE_EXTRACT_MAX_PMATCH];
+ int bMustFree;
+ es_str_t *estr;
+ char *str;
+ struct var r[CNFFUNC_MAX_ARGS];
+ int iLenBuf;
+ unsigned iOffs;
+ short iTry = 0;
+ uchar bFound = 0;
+ iOffs = 0;
+ sbool bHadNoMatch = 0;
+ short i;
+ char submatchStr[3]; /* just enough to place 0..49 */
+ char *cstr;
+ struct json_object *json;
+ char saveChar;
+
+ cnfexprEval(func->expr[0], &r[0], usrptr);
+ /* search string is already part of the compiled regex, so we don't
+ * need it here!
+ */
+ cnfexprEval(func->expr[2], &r[2], usrptr);
+ str = (char*) var2CString(&r[0], &bMustFree);
+ matchnbr = (short) var2Number(&r[2], NULL);
+
+ /* first see if we find a match, iterating through the series of
+ * potential matches over the string.
+ */
+ while(!bFound) {
+ int iREstat;
+ iREstat = regexp.regexec(func->funcdata, (char*)(str + iOffs),
+ RE_EXTRACT_MAX_PMATCH, pmatch, 0);
+ dbgprintf("re_extract: regexec return is %d\n", iREstat);
+ if(iREstat == 0) {
+ if(pmatch[0].rm_so == -1) {
+ dbgprintf("oops ... start offset of successful
regexec is -1\n");
+ break;
+ }
+ if(iTry == matchnbr) {
+ bFound = 1;
+ } else {
+ dbgprintf("re_extract: regex found at offset
%d, new offset %d, tries %d\n",
+ iOffs, (int) (iOffs +
pmatch[0].rm_eo), iTry);
+ iOffs += pmatch[0].rm_eo;
+ ++iTry;
+ }
+ } else {
+ break;
+ }
+ }
+ dbgprintf("re_extract: regex: end search, found %d\n", bFound);
+ if(!bFound) {
+ bHadNoMatch = 1;
+ goto finalize_it;
+ }
+ /* extract all submatches */
+ ret->datatype = 'J';
+ ret->d.json = json_object_new_object();
+ if (ret->d.json == NULL) {
+ DBGPRINTF("re_extract() unable to create json object\n");
+ bHadNoMatch = 1;
+ goto finalize_it;
+ }
+ for (i = 0; i < RE_EXTRACT_MAX_PMATCH; i++) {
+ if (pmatch[i].rm_so == -1) {
+ break;
+ }
+ iLenBuf = pmatch[i].rm_eo - pmatch[i].rm_so;
+ cstr = str + iOffs + pmatch[i].rm_so;
+ saveChar = *(cstr + iLenBuf);
+ *(cstr + iLenBuf) = '\0';
+ DBGPRINTF("re_extract() sub %d '%s'\n", i, cstr);
+ json = json_object_new_string(cstr);
+ *(cstr + iLenBuf) = saveChar;
+ if (json == NULL) {
+ DBGPRINTF("re_extract() unable to create json from
string\n");
+ bHadNoMatch = 1;
+ goto finalize_it;
+ }
+ snprintf(&submatchStr[0], sizeof(submatchStr), "%d", i);
+ json_object_object_add(ret->d.json, &submatchStr[0], json);
+ }
+
+finalize_it:
+ if(bMustFree) free(str);
+ varDelete(&r[0]);
+ varDelete(&r[2]);
+ if(bHadNoMatch) {
+ cnfexprEval(func->expr[3], &r[3], usrptr);
+ estr = var2String(&r[3], &bMustFree);
+ /* Note that we do NOT free the string that was returned/created
+ * for r[3]. We pass it to the caller, which in turn frees it.
+ * This saves us doing one unnecessary memory alloc & write.
+ */
+ ret->datatype = 'S';
+ ret->d.estr = estr;
+ }
+ return;
+}
+
/* Perform a function call. This has been moved out of cnfExprEval in order
* to keep the code small and easier to maintain.
@@ -1497,7 +1586,7 @@ doFuncCall(struct cnffunc *func, struct var *ret, void*
usrptr)
}
ret->datatype = 'S';
if(bMustFree) es_deleteStr(estr);
- varFreeMembers(&r[0]);
+ varDelete(&r[0]);
free(str);
break;
case CNFFUNC_TOLOWER:
@@ -1508,7 +1597,7 @@ doFuncCall(struct cnffunc *func, struct var *ret, void*
usrptr)
es_tolower(estr);
ret->datatype = 'S';
ret->d.estr = estr;
- varFreeMembers(&r[0]);
+ varDelete(&r[0]);
break;
case CNFFUNC_CSTR:
cnfexprEval(func->expr[0], &r[0], usrptr);
@@ -1517,7 +1606,7 @@ doFuncCall(struct cnffunc *func, struct var *ret, void*
usrptr)
estr = es_strdup(estr);
ret->datatype = 'S';
ret->d.estr = estr;
- varFreeMembers(&r[0]);
+ varDelete(&r[0]);
break;
case CNFFUNC_CNUM:
if(func->expr[0]->nodetype == 'N') {
@@ -1528,7 +1617,7 @@ doFuncCall(struct cnffunc *func, struct var *ret, void*
usrptr)
} else {
cnfexprEval(func->expr[0], &r[0], usrptr);
ret->d.n = var2Number(&r[0], NULL);
- varFreeMembers(&r[0]);
+ varDelete(&r[0]);
}
ret->datatype = 'N';
break;
@@ -1546,11 +1635,14 @@ doFuncCall(struct cnffunc *func, struct var *ret, void*
usrptr)
}
ret->datatype = 'N';
if(bMustFree) free(str);
- varFreeMembers(&r[0]);
+ varDelete(&r[0]);
break;
case CNFFUNC_RE_EXTRACT:
doFunc_re_extract(func, ret, usrptr);
break;
+ case CNFFUNC_RE_EXTRACT_HASH:
+ doFunc_re_extract_hash(func, ret, usrptr);
+ break;
case CNFFUNC_FIELD:
cnfexprEval(func->expr[0], &r[0], usrptr);
cnfexprEval(func->expr[1], &r[1], usrptr);
@@ -1579,9 +1671,9 @@ doFuncCall(struct cnffunc *func, struct var *ret, void*
usrptr)
}
ret->datatype = 'S';
if(bMustFree) free(str);
- varFreeMembers(&r[0]);
- varFreeMembers(&r[1]);
- varFreeMembers(&r[2]);
+ varDelete(&r[0]);
+ varDelete(&r[1]);
+ varDelete(&r[2]);
break;
case CNFFUNC_PRIFILT:
pPrifilt = (struct funcData_prifilt*) func->funcdata;
@@ -1688,8 +1780,8 @@ evalStrArrayCmp(es_str_t *estr_l, struct cnfarray* ar,
int cmpop)
}
#define FREE_BOTH_RET \
- varFreeMembers(&r); \
- varFreeMembers(&l)
+ varDelete(&r); \
+ varDelete(&l)
#define COMP_NUM_BINOP(x) \
cnfexprEval(expr->l, &l, usrptr); \
@@ -1716,9 +1808,9 @@ evalStrArrayCmp(es_str_t *estr_l, struct cnfarray* ar,
int cmpop)
#define FREE_TWO_STRINGS \
if(bMustFree) es_deleteStr(estr_r); \
- if(expr->r->nodetype != 'S' && expr->r->nodetype != 'A')
varFreeMembers(&r); \
+ if(expr->r->nodetype != 'S' && expr->r->nodetype != 'A')
varDelete(&r); \
if(bMustFree2) es_deleteStr(estr_l); \
- varFreeMembers(&l)
+ varDelete(&l)
/* evaluate an expression.
* Note that we try to avoid malloc whenever possible (because of
@@ -1769,7 +1861,7 @@ cnfexprEval(struct cnfexpr *expr, struct var *ret, void*
usrptr)
if(bMustFree)
es_deleteStr(estr_r);
}
}
- varFreeMembers(&r);
+ varDelete(&r);
}
} else if(l.datatype == 'J') {
estr_l = var2String(&l, &bMustFree);
@@ -1791,7 +1883,7 @@ cnfexprEval(struct cnfexpr *expr, struct var *ret, void*
usrptr)
if(bMustFree)
es_deleteStr(estr_r);
}
}
- varFreeMembers(&r);
+ varDelete(&r);
}
if(bMustFree) es_deleteStr(estr_l);
} else {
@@ -1808,9 +1900,9 @@ cnfexprEval(struct cnfexpr *expr, struct var *ret, void*
usrptr)
} else {
ret->d.n = (l.d.n == r.d.n); /*CMP*/
}
- varFreeMembers(&r);
+ varDelete(&r);
}
- varFreeMembers(&l);
+ varDelete(&l);
break;
case CMP_NE:
cnfexprEval(expr->l, &l, usrptr);
@@ -2038,9 +2130,9 @@ cnfexprEval(struct cnfexpr *expr, struct var *ret, void*
usrptr)
ret->d.n = 1ll;
else
ret->d.n = 0ll;
- varFreeMembers(&r);
+ varDelete(&r);
}
- varFreeMembers(&l);
+ varDelete(&l);
break;
case AND:
cnfexprEval(expr->l, &l, usrptr);
@@ -2051,17 +2143,17 @@ cnfexprEval(struct cnfexpr *expr, struct var *ret,
void* usrptr)
ret->d.n = 1ll;
else
ret->d.n = 0ll;
- varFreeMembers(&r);
+ varDelete(&r);
} else {
ret->d.n = 0ll;
}
- varFreeMembers(&l);
+ varDelete(&l);
break;
case NOT:
cnfexprEval(expr->r, &r, usrptr);
ret->datatype = 'N';
ret->d.n = !var2Number(&r, &convok_r);
- varFreeMembers(&r);
+ varDelete(&r);
break;
case 'N':
ret->datatype = 'N';
@@ -2112,7 +2204,7 @@ cnfexprEval(struct cnfexpr *expr, struct var *ret, void*
usrptr)
cnfexprEval(expr->r, &r, usrptr);
ret->datatype = 'N';
ret->d.n = -var2Number(&r, &convok_r);
- varFreeMembers(&r);
+ varDelete(&r);
break;
case 'F':
doFuncCall((struct cnffunc*) expr, ret, usrptr);
@@ -2150,6 +2242,7 @@ cnffuncDestruct(struct cnffunc *func)
switch(func->fID) {
case CNFFUNC_RE_MATCH:
case CNFFUNC_RE_EXTRACT:
+ case CNFFUNC_RE_EXTRACT_HASH:
if(func->funcdata != NULL)
regexp.regfree(func->funcdata);
break;
@@ -3363,6 +3456,13 @@ funcName2ID(es_str_t *fname, unsigned short nParams)
return CNFFUNC_INVALID;
}
return CNFFUNC_RE_EXTRACT;
+ } else if(!es_strbufcmp(fname, (unsigned char*)"re_extract_hash",
sizeof("re_extract_hash") - 1)) {
+ if(nParams != 4) {
+ parser_errmsg("number of parameters for re_extract()
must be four "
+ "but is %d.", nParams);
+ return CNFFUNC_INVALID;
+ }
+ return CNFFUNC_RE_EXTRACT_HASH;
} else if(!es_strbufcmp(fname, (unsigned char*)"field", sizeof("field")
- 1)) {
if(nParams != 3) {
parser_errmsg("number of parameters for field() must be
three "
@@ -3503,6 +3603,7 @@ cnffuncNew(es_str_t *fname, struct cnffparamlst* paramlst)
switch(func->fID) {
case CNFFUNC_RE_MATCH:
case CNFFUNC_RE_EXTRACT:
+ case CNFFUNC_RE_EXTRACT_HASH:
/* need to compile the regexp in param 2, so
this MUST be a constant */
initFunc_re_match(func);
break;
@@ -3619,6 +3720,10 @@ cnfDoInclude(char *name)
return 0;
}
+/* frees struct var members, but not the struct itself. This is because
+ * it usually is allocated on the stack. Callers who dynamically allocate
+ * struct var need to free the struct themselfes!
+ */
void
varDelete(struct var *v)
{
@@ -3630,7 +3735,17 @@ varDelete(struct var *v)
cnfarrayContentDestruct(v->d.ar);
free(v->d.ar);
break;
- default:break;
+ /* We must decrement refcount, as we had incremented it while acquiring
+ * the object. It is needed to properly free temporary JSON-based
variables.
+ */
+ case 'J':
+ json_object_put(v->d.json);
+ break;
+ case 'N':
+ break;
+ default:
+ dbgprintf("warning: trying to delete a variable of unknown type
'%c'\n", v->datatype);
+ break;
}
}
diff --git a/grammar/rainerscript.h b/grammar/rainerscript.h
index 001dff4..c61a887 100644
--- a/grammar/rainerscript.h
+++ b/grammar/rainerscript.h
@@ -233,6 +233,7 @@ enum cnffuncid {
CNFFUNC_CNUM,
CNFFUNC_RE_MATCH,
CNFFUNC_RE_EXTRACT,
+ CNFFUNC_RE_EXTRACT_HASH,
CNFFUNC_FIELD,
CNFFUNC_PRIFILT,
CNFFUNC_LOOKUP
diff --git a/runtime/msg.c b/runtime/msg.c
index f634329..d173644 100644
--- a/runtime/msg.c
+++ b/runtime/msg.c
@@ -2613,6 +2613,7 @@ msgGetJSONPropJSON(msg_t *pMsg, msgPropDescr_t *pProp,
struct json_object **pjso
if(*pjson == NULL) {
ABORT_FINALIZE(RS_RET_NOT_FOUND);
}
+ json_object_get(*pjson);
finalize_it:
if(pProp->id == PROP_GLOBAL_VAR)
--
1.7.9.5
_______________________________________________
rsyslog mailing list
http://lists.adiscon.net/mailman/listinfo/rsyslog
http://www.rsyslog.com/professional-services/
What's up with rsyslog? Follow https://twitter.com/rgerhards
NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a myriad of
sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you DON'T LIKE
THAT.
