use dynamic sized command line buffer #431
Conversation
Previously we used a fixed size 8k buffer when parsing command line arguments. This sounds much, but there are command lines bigger than 8k and they are simply cut off without any warning. Instead we use a dynamic sized buffer with the size of the raw command now.
| /* get the command arguments */ | ||
| if (cmd != NULL) { | ||
| if (cmd == NULL) { | ||
| log_debug_info(DEBUGL_COMMANDS | DEBUGL_CHECKS | DEBUGL_MACROS, 2, "Expanded Command Output: %s\n", *full_command); |
There was a problem hiding this comment.
In this case, it means that there was no need to further expand the command? I.e for commands like DISABLE_NOTIFICATIONS where there are no args
There was a problem hiding this comment.
sorry, this is not external commands, but check commands... Similar point though.
There was a problem hiding this comment.
tbh, i didn't check where this is used, i just inverted the huge if clause to return early instead of having the hole function in a huge if. The functionality did not change at this point.
I should have put this in a separate commit.
There was a problem hiding this comment.
My C is probably already rusty, but don't we just put a single, un-expanded check argument into the temp_arg variable?
I.e if we have example_command!$_MY_MACRO$ we end up putting $_MY_MACRO$ into temp_arg and passing it to process_macros_r which puts the expanded version of the macro in arg_buffer ?
|
that's right, but in my case i had command with very long arguments like |
|
You had a single argument that was above 8k bytes!? |
|
right, basically a tarball of a folder as base64 encoded string :-) I know, not the usual usecase, but naemon should either work or throw an error :-) |
There was a problem hiding this comment.
Jeez OK! Seems sensible enough. I guess we might even save some bytes on normal smaller commands like this.
When I first read the title I figured it would be the total check_command (expanded/non expanded) that we change, so I guess just make sure to note in the changelog that it's the size of the individual command line args that can now be longer.
Honestly, I was also testing the total check command length first^^ |
|
sorry for the confusion :-) thanks for the review. |
Previously we used a fixed size 8k buffer when parsing command line arguments. This sounds much, but there are command lines bigger than 8k and they are simply cut off without any warning.
Instead we use a dynamic sized buffer with the size of the raw command now.
(while on it, fixing the crazy indent)