Add more descriptive error strings for common errors, namely BCERR_INVALID_INPUT, BCERR_NO_FORMAT_FILE, and BCERR_NO_MATCHING_FORMAT.
In bc_strerror, move the return statement to the line below the case statement for each case. These are purely whitespace changes; there is no functional change. This change is being made in preparation for the next change, which will add to the existing error strings. It is preferable to keep these as separate changes so that it's easier to see which error strings are added to.
The formats file is now located at "formats.txt" instead of "formats". This change was made to make it easier for Windows users to modify the formats file. Double-clicking on the formats file in Windows will now open it in Notepad. Developers on Windows should turn on the "core.autocrlf" option in git: git config --global core.autocrlf true This will cause the checked out files to use Windows-style newlines, which Notepad can read. Without this option, the newlines will be Unix-style and Notepad will not read them correctly. Although the formats file could have been stored in the repository with Windows-style newlines, that option is not feasible because the libbitconvert codebase cannot handle such a formats file when compiled on a Unix-like system. However, the libbitconvert codebase works with such a formats file when compiled with the MinGW compiler on Windows. As a result, storing the formats file with Unix-style newlines and having git deal with the newline conversion on Windows is the most feasible option.
The BCERR_INVALID_TRACK error code is no longer used. It was phased out when libbitconvert switched to accepting all tracks at once rather than accepting each track individually, in which case the track number was specified for each track. This change was made in commit 7d77d2d.
BCERR_RESULT_FULL is no longer used because the result fields are dynamically allocated.
Prior to this fix, if the library user called bc_decode and then bc_decoded_free, the library would segfault because result->field_values was not initialized and the library tried to free it. This bug had been hidden because all library users so far have called bc_find_fields after bc_decode, which initializes result->field_values. This fix checks result->field_names before freeing other result->field_* values. This works because result->field_names is always initialized.
"Do we need to skip?" check was 'k + 1 < num_fields'; this incorrectly decides NOT to skip when we get a RE match fail with precisely one field that needs to be skipped.
Added space after comma in function calls Moved initializations-at-declaration to a chunk of initializations after the declarations.
Replaced '*buf = realloc...' with 't = realloc...', but forgot to replace 'if (NULL == *buf)' with 'if (NULL == t)'.
Now uses a temporary for realloc's return value, to avoid leaking memory on realloc failure.
Now always moves current file position past the current set of track fields, and cleanly deallocates memory that we still have pointers to, for every possible failure. (Fixing realloc leaks will be #2.)
Changed type of variables tracking memory sizes into size_t
ovector, the vector passed into pcre_exec that holds the offsets of the captured substrings, is now dynamically allocated. It is grown until pcre_exec returns non-zero, indicating that ovector was large enough or an error occurred. This change eliminates MAX_CAPTURED_SUBSTRINGS, which is no longer needed now that ovector is dynamically-sized. This change is the last one needed to make libbitconvert completely free of static buffers. The test drivers (driver and combine) still have static buffers, but the library itself does not.
Added array growing code to bc_decode_track_fields and a fields_size parameter to the bc_decode_track_fields interface for passing the current allocated size of the fields lists. Added initial allocation of the fields lists in bc_decode_fields. Updated bc_decode to initialize field_names to NULL, which is the new correct way to denote empty fields lists. Added corresponding frees in bc_decoded_free. This change also removes BC_NUM_FIELDS, which is no longer needed because there are no statically-sized fields in struct bc_decoded anymore. There is still the statically-sized ovector to fix in bc_decode_track_fields, though.
This is purely a coding style update. Most of it is moving braces to be consistent with the other code. Some braces were also added for clarity.
The emptying of field_names has been removed because it is unneeded: when control reaches the bottom of the bc_decode_fields loop, field_names is guaranteed to be empty already because rc is BCINT_NO_MATCH, which means that no field data was added to the field lists because bc_decode_track_fields has not started adding field data when it returns BCINT_NO_MATCH.
Each field_name (one element of field_names) is now dynamically allocated. This allows the field name to be copied via strcpy directly into the field_name. The field_names array is now NULL-terminated. All lines of code that depended on the previous behavior (empty string-terminated list), including one in the test driver, have been updated with this new assumption. This change also removes BC_FIELD_SIZE, which is no longer needed because there are no more statically-sized string buffers, only statically-sized arrays that store strings.
This change was fairly simple because pcre_get_named_substring returns an already-allocated string, which can be assigned directly to the field_value (an element of field_values). The appropriates frees were added to bc_decoded_free.
Check the value of pcre_get_named_substring's return code to see if there were problems and return an error if there were. Previously, the return code of pcre_get_named_substring was ignored, which could have led to segfaults or other problems given a malformed formats file.
The return code for pcre_exec is now stored in exec_rc instead of rc. This will improve readability and correctness when other functions start using rc, which will be used more locally than exec_rc.
In bc_decode_fields, the result name is now set by assigning a pointer instead of strncpy. This change also moves that assignment into the if block where a match has been found in order to prevent unnecessary operations. Added initialization of name in bc_decode to ensure name has a value set even if the user doesn't run bc_find_fields. This is necessary so that bc_decoded_free will work in that case.
To implement this update, the following changes were made: - bc_decode_format interface changed to accept char** and drop length - bc_decode_format performs memory allocation for decoded track data - BC_*DECODED_SIZE were removed - bc_decoded_free was added to free the allocated decoded track data - driver now checks for NULL decoded track data (previously unneeded) As a side effect of updates to bc_decode to use the new bc_decode_format interface, the code for accepting input data was also made more robust by adding checks for NULL input track data.
Change result_len bc_decode_format parameter for Track 3 call from BC_T1_DECODED_SIZE to BC_T3_DECODED_SIZE. This was an error during copy and paste when the code was first created.
This change replaces the last of use fgets (which was in bc_decode_track_fields) with dynamic_fgets. As a side effect, the FORMAT_LEN constant is no longer needed because the library accepts lines in the format file of any length due to the use of dynamic_fgets. Instead of using buf and buf2, which had been added for the first use of dynamic_fgets, bc_decode_track_fields now uses only buf since the buffer is handled by dynamic_fgets throughout the function. As a result, the buffer's space can be reused, which reduces the number of allocations and frees required. Because of the dynamically-sized buffer, strcpy calls have been changed to strncpy for copying into field_names and field_values to prevent overflowing the destination buffers, which have not yet been updated to be dynamically-sized.
Instead of using rc to store dynamic_fgets' return code, use fgets_rc. This eliminates potential confusion with the use of the rc variable in bc_decode_track_fields, which is later used to refer to the return code of pcre_exec.
Implement suggestions from Dave Vandervies to make the field name finding code simpler and more readable. As a side effect, two local variables were eliminated. The only functional change here is that a space between the period and field name is now enforced. Since this was never relied-on in the formats file, no changes are needed there. Aside from that, the new code appears to be functionally-equivalent to the old code.
When dynamic_fgets was introduced (in commit 0406721), a malloc fail check for buf2 was missed. This patch adds the missing check.
The name variable in bc_decode_fields is now dynamically-sized through dynamic_fgets. This change makes use of dynamic_fgets' reusable buffers feature in skipping to the next card specification.
dynamic_fgets cannot use the same interface as fgets because dynamic_fgets has more error conditions than fgets and because the location of the buffer may change when it is reallocated. Previously, the extra error condition was ignored and the buffer location was assumed not to change, which worked in small test cases but would probably fail eventually. The dynamic_fgets interface has been updated to return an int for error conditions and to take a char** for the buffer so that a new buffer location (possible after realloc) can be returned to the caller. For now, the error conditions are out of memory and EOF found. dynamic_fgets has also been changed to take an int* instead of an int for the buffer size. This is so that the caller knows what the buffer's new size is. This is not strictly necessary (the caller doesn't need to know this), but it is useful in that it allows multiple calls to dynamic_fgets using the same buffer. Reusing the buffer allows for fewer memory allocation and free operations.
The dynamic_fgets function is functionally-equivalent to fgets except it grows the passed-in buffer as necessary and continues copying the line contents into the buffer until a newline is reached. Updated bc_decode_track_fields to use dynamic_fgets for the track description line, which contains the encoding type and regular expression. This is read into the newly-created buf2. Eventually all fgets operations will be migrated to dynamic_fgets.
This patch fixes the following warning from GCC 3.4.2 on Solaris by adding an appropriate cast (to int) for isspace: bitconvert.c:197: warning: subscript has type `char'
Instead of being statically-allocated in the library, the memory for struct bc_input is now managed by the user of the library. The test drivers have been updated to reflect this; they each allocate their own static buffers which are passed into the struct bc_input. As a side-effect, bc_combine now allocates memory since it is the owner of the struct bc_input "combined". The user of bc_combine must thus free the output using the new function bc_input_free. Because of the change, bc_init no longer accepts a struct bc_input because no initialization is required anymore. This also eliminates the meaning ambiguity with the function name -- does it mean initialize a struct bc_input or initialize the library? It now means initialize the library.
bc_combine combines the data from forward and backward swipes of a particular card. Currently only track 2 is combined for testing purposes. A driver for bc_combine was added (combine.c). It accepts a set of forward swipes in the first 3 lines of standard in, followed by a set of backward swipes in the second 3 lines of standard in. BCERR_UNIMPLEMENTED was added to the error codes for referring to unimplemented behavior in bc_combine. Testing the bc_combine driver on 16 different cards showed that the swipes from 13 cards could be successfully combined, while the swipes of 3 cards could not. This should improve with future changes to bc_combine.
The name of the to_character function did not accurately reflect what the function does: convert bits into ASCII. It was confusing because the input and the output were both of the char (character) type. As a result, the name has been changed to to_ascii based on a suggestion from Stephen Paul Weber.