param from string

[goatshriek]

The Stumpless Rust crate and CLI tool creates targets and entries to do whatever logging needed. In order to provide CLI support, many of these tasks need to use the strings provided in the command to set up stumpless structures. One of these is the string used to create structured data params of the logged entry. To support this, a new function is needed named stumpless_new_param_from_string which parses a string and attempts to create a param from it.

General Approach

There are a few details left out of the following approach, for you to fill in as you encounter them. If you find you need help, please ask here or on the project gitter and someone can help you get past the stumbling block.

First read the relevant section of RFC 5424 and the logger manpage section for the --sd-param option. The RFC describes what characters are valid in param names and values. These are already validated by the stumpless_new_param function, so you will be best served by simply using this function to create your new param to take advantage of the checks. The manpage describes the string that is used to create params. You will expect strings in this same format: name="value".

Optionally, you may review the current Rust implementation of this functionality for the CLI tool. This change will move this functionality into the C library so that other language bindings such as C++ can offer the same functionality.

Next, read this section of the development documentation on adding new functions. Once you understand how this process works, start defining the new function.

Your new function will be named stumpless_new_param_from_string, should take a single string parameter, and should return a new stumpless_param structure based on the string. In the event of an error (such as a NULL or invalid string), return NULL and raise an invalid param error STUMPLESS_INVALID_PARAM. You'll need to add this new error type to include/stumpless/error.h. Declare your new function include/stumpless/param.h and define it in the corresponding source file src/param.c.

Add tests for your new function in the existing test suite in test/function/param.cpp. Be sure that you test for both success and error cases, particularly escaped characters.

[panosfol]

Hello! I would like to take a shot at this!

[goatshriek]

Sure thing! I will assign it so that others know it is in work.

[panosfol]

@goatshriek should this feature be implemented against the v2.1.0 version?

[goatshriek]

This should be implemented against the latest branch, and will be included in release 2.2.0 later this year.

[panosfol]

@goatshriek Should i use the same regex pattern "(.*)="(.*)"" (translated to C) from the rust implementation or create a new one? I think this doesn't account for the case of multiple equal signs between the name and the "value" for example.
Maybe something like "^[a-zA-Z0-9]+=\"[a-zA-Z0-9]+\"$" would work better?

Also do i need to add an error for when the regex does not compile corectly? I.e the regcomp function returns an error.

[goatshriek]

Don't use an actual regular expression - this isn't worth the added dependency, and it will not be as performant as a custom scanner.

I recommend your code iterate through the string looking for the first equal sign, storing the length. Skip the equal sign, the first quote, and scan to the end, making sure it is another quote, again storing the length. At the end, you'll have the position and length of the name and value, and can pass them to the param constructor.

[panosfol]

Don't use an actual regular expression - this isn't worth the added dependency, and it will not be as performant as a custom scanner.

I recommend your code iterate through the string looking for the first equal sign, storing the length. Skip the equal sign, the first quote, and scan to the end, making sure it is another quote, again storing the length. At the end, you'll have the position and length of the name and value, and can pass them to the param constructor.

Yes thats what i did. I wanted to use regex to check if the format of the string is the correct one. Im parsing the string exactly as you suggested.

[goatshriek]

Ah, thank you for clarifying. In the tests, I would recommend that you use a direct string comparison instead of a regular expression for format checking, so that you don't have to worry about these kinds of errors. If you want to use a regular expression, take a look at TestRFC5424Compliance in test/helper/rfc5424.cpp, which uses regular expressions for some tests.

[panosfol]

Ah, thank you for clarifying. In the tests, I would recommend that you use a direct string comparison instead of a regular expression for format checking, so that you don't have to worry about these kinds of errors. If you want to use a regular expression, take a look at TestRFC5424Compliance in test/helper/rfc5424.cpp, which uses regular expressions for some tests.

Ill use direct string comparison, can you clarify a bit on how to implement that? Just to make sure.
Also shouldn't i check about the format of the string in the stumpless_new_param_from_string() before i parse?

[goatshriek]

The EXPECT_STREQ macro will perform a direct string comparison in tests, it's used in many of the unit tests already.

The most efficient way to validate the format is while scanning, and simply raise an error and exit if it doesn't match. For example, if you encounter an illegal character, simply raise the invalid param error and return NULL. Same if the first = character is not directly followed by a " character, etc.

[panosfol]

@goatshriek Oh ok thank you very much, I believe Ill have it ready soon!