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

buffer optimization: long comments do not trigger errorBufferTooSmall #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dzzie
Copy link

@dzzie dzzie commented Dec 15, 2015

this should do the trick give it a try.

@dzzie
Copy link
Author

dzzie commented Dec 15, 2015

actually one note on this, the prototype should be moved to the private: section of the .h its not for public consumption as it expects that a read of len has already occured

@stevemarple
Copy link
Owner

It fails the regression test. Values which are expected to be read from test.ini are not found. Below is the diff that make regressiontest outputs:

Using file test.ini
   File open? true
     Looking for key "mac"
-      Value of mac is "01:23:45:67:89:AB"
+      Error: key not found (6)
     Looking for key "mac" in section "network"
-      Value of mac is "01:23:45:67:89:AB"
+      Error: section not found (5)
     Looking for key "mac" in section "network2"
-      Value of mac is "ee:ee:ee:ee:ee:ee"
+      Error: section not found (5)
     Looking for key "mac" in section "fake"
       Error: section not found (5)
     Looking for key "ip"
-      Value of ip is "192.168.1.2"
+      Error: key not found (6)
     Looking for key "gateway"
       Value of gateway is "192.168.1.1"
     Looking for key "hosts allow" in section "network"
-      Value of hosts allow is "example.com"
+      Error: section not found (5)
     Looking for key "hosts allow" in section "network"
-      Value of hosts allow is "example.com"
+      Error: section not found (5)
     Looking for key "hosts allow" in section "network2"
-      Value of hosts allow is "sloppy.example.com"
+      Error: section not found (5)
     Looking for key "hosts allow" in section "network2"
-      Value of hosts allow is "sloppy.example.com"
+      Error: section not found (5)
     Looking for key "string" in section "misc"
-      Value of string is "123456789012345678901234567890123456789001234567890"
+      Error: section not found (5)
     Looking for key "string2" in section "misc"
-      Value of string2 is "a string with spaces in it"
+      Error: section not found (5)
     Looking for key "pi" in section "misc"
-      Value of pi is "3.141592653589793"
-    Pi: 3.14159
+      Error: section not found (5)
+    Could not read "pi" from section "misc" in test.ini
+      Error: section not found (5)
 ----
 Done
make: *** [regressiontest] Error 1

(Lines beginning with "-" are the reference output, lines which begin with "+" are from the current output.)

@dzzie
Copy link
Author

dzzie commented Dec 15, 2015

sorry I just realized i had a bug in this on the way home from the grocery store, one sec

@dzzie
Copy link
Author

dzzie commented Dec 15, 2015

ok I amended the commit, I realized I needed to scan the existing buffer for \n before scanning ahead, I was testing on a truncated ini, this time I ran it through the full set sorry

@stevemarple
Copy link
Owner

That looks good, it passes the regression tests now.

I noticed that I haven't made the library available for automatic installation via the IDE. I plan to update my master copy and submit for inclusion on the lists of third-party libraries. Then I'll merge your changes and push an updated version. That way users can revert if necessary.
Thanks for your contribution, Steve.

@dzzie
Copy link
Author

dzzie commented Dec 15, 2015

one other thought I just had...I wonder if I need to pass the i counter into the readUntilNewLine function and start from there in the for loop instead of i=0,

since the buffer has already been partially processed and pos++ that many times already.

nothing showed up in the tests, but I will see if I can construct one. Simple as the logic seems Its kind of tricky to nail down just right :-\

edit: I cant seem to construct a scenario that bugs it out, the change didnt hurt anything either. The data i thought that might trigger it would have been a comment line with space preceeding the comment char, followed immediately by setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants