On Tue, Dec 29, 2015 at 06:38:31PM -0500, Alex Xu wrote: > I was attempting to configure sqlite3 output in ulogd and could not > determine the function of the "buffer" configuration option. > > $ grep buffer output/sqlite3/ulogd_output_sqlite3.c > int buffer_size; > int buffer_curr; > .key = "buffer", > #define buffer_ce(pi) (pi)->config_kset->ces[2].u.value > priv->buffer_curr++; > /* initialize our buffer size and counter */ > priv->buffer_size = buffer_ce(pi); > priv->buffer_curr = 0; > $ grep -rF buffer_curr . > output/sqlite3/ulogd_output_SQLITE3.c: int buffer_curr; > output/sqlite3/ulogd_output_SQLITE3.c: priv->buffer_curr++; > output/sqlite3/ulogd_output_SQLITE3.c: priv->buffer_curr = 0; > > Now, it is quite likely that I am missing something, but it would > appear that buffer_curr is never actually *read*, only *written*, > and it's a basic fact of programming that a variable can have no effect > if it is never read from. [citation needed] > > It would appear that the original code in ulogd-1.x employed a 'buffer' > which was actually a counter for when to tell sqlite to sync to disk. > When the code was rewritten by "Holger" and committed by Pablo Neira > Ayuso (8f7bb61), the syncing part was removed but the buffer variable > was simply renamed. > > Nobody noticed because one fsync every few seconds is tiny. > > Therefore, I think we should remove the "buffer" option entirely. > > Thoughts? I will send a patch if there are no objections. This may be well a leftover that was introduced by when that large rewrite took, please submit a patch that we can review. I'm also Cc'ing Eric Leblond, he maintains ulogd2, just in case he finds some cycles to confirm this issue. Thanks for reporting. -- To unsubscribe from this list: send the line "unsubscribe netfilter" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
