Skip to content
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.

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

stats.c: Treemap - May not handle paths with commas #58

Closed
dpieski opened this issue Jun 3, 2020 · 4 comments
Closed

stats.c: Treemap - May not handle paths with commas #58

dpieski opened this issue Jun 3, 2020 · 4 comments
Labels
bug Something isn't working stats Stats step

Comments

@dpieski
Copy link
Contributor

dpieski commented Jun 3, 2020

sist2 version: 2.4.0

Platform (Linux or Docker): Docker

Elasticsearch version: 7.7.0

In stats.c you have the function csv_escape.
in csv_escape, you
Check if there is a comma or doublequote:
if there isn't, you just pass along the string.
If there is a doublequote, you cycle through the string and double up the doublequotes.

But you don't do anything if there is a comma in the string. I think you should have to escape strings with commas too.

Also, if I remember correctly (and its been like 15yrs since I took C), counting starts at 0 in C and while loops check the condition before executing the loop, so I think your while loop is cutting off the first character of the string since it will not add the 'zeroth' character of the string to *out

@dpieski
Copy link
Contributor Author

dpieski commented Jun 3, 2020

Also couldn't you replace the while loop with something like:

snprintf(dst, PATH_MAX *4, "\"%s\"", str)

Or to the if (rfind(str, ',') .... put the above in an else?

@dpieski
Copy link
Contributor Author

dpieski commented Jun 3, 2020

Example from treemap.csv

image

See the different colors of the commas from VSC showing the different columns.
See also, the row with the commas has lost its first character "6"

@simon987 simon987 added bug Something isn't working stats Stats step labels Jun 3, 2020
simon987 added a commit that referenced this issue Jun 5, 2020
@simon987
Copy link
Collaborator

simon987 commented Jun 5, 2020

Yeah. I meant to surround by double-quotes in the "else" part, it must have been very late when I wrote it, sorry about that. It should be fixed in v2.4.1

@dpieski
Copy link
Contributor Author

dpieski commented Jun 22, 2020

This appears corrected. Closing.

@dpieski dpieski closed this as completed Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stats Stats step
Projects
None yet
Development

No branches or pull requests

2 participants