[GNC-dev] gnucash maint: Bug 796982 - Import Bills & Invoices: change in un_escape() routine...

[John Ralls]

> On Dec 19, 2018, at 2:04 AM, Geert Janssens <geert.gnucash at kobaltwit.be> wrote:
> 
> Op woensdag 19 december 2018 06:32:16 CET schreef John Ralls:
>> Updated	 via  https://github.com/Gnucash/gnucash/commit/f2976420 (commit)
>> 	from  https://github.com/Gnucash/gnucash/commit/2524482b (commit)
>> 
>> 
>> 
>> commit f29764202ec9f7ace6eb726c246799cb19ad5c1a
>> Author: John Ralls <jralls at ceridwen.us>
>> Date:   Tue Dec 18 21:29:53 2018 -0800
>> 
>>    Bug 796982 - Import Bills & Invoices: change in un_escape() routine...
>> 
>>    causes description and notes fields to be mangled.
>> 
>>    Simple error, but rewrote the function to be more idiomatic, resisting
>>    temptation to abuse the ternary operator.
>> 
>> diff --git a/gnucash/import-export/bi-import/dialog-bi-import.c
>> b/gnucash/import-export/bi-import/dialog-bi-import.c index 5b46e84..6aabcc3
>> 100644
>> --- a/gnucash/import-export/bi-import/dialog-bi-import.c
>> +++ b/gnucash/import-export/bi-import/dialog-bi-import.c
>> @@ -876,30 +876,26 @@ gnc_bi_import_create_bis (GtkListStore * store,
>> QofBook * book, * @param char* String to be modified
>>  * @return char* Modified string.
>> */
>> -static char * un_escape(char *str)
>> +static char*
>> +un_escape(char *str)
>> {
>>     gchar quote = '"';
>>     gchar *newStr = NULL, *tmpstr = str;
>>     int n = 0;
>> +
>>     newStr = g_malloc(strlen(str)+1); // This must be freed in the calling
>> code. while(*tmpstr != '\0')
>>     {
>>         if(*tmpstr == quote)
>> -        {
>> -            tmpstr++;
>> -            if(*tmpstr == quote)
>> -            {
>> -                newStr[n] = quote;
>> -            }
>> -        }
>> +            // We always want the character after a quote.
>> +            newStr[n] = *(++tmpstr);
>>         else
>> -        {
>> -            newStr[n] = *str;
>> -        }
>> -            tmpstr++;
>> -            n++;
>> +            newStr[n] = *tmpstr;
>> +        ++tmpstr;
>> +        ++n;
>>     }
>> +
>>     g_free (str);
>> -	newStr[n] = '\0'; //ending the character array
>> +    newStr[n] = '\0'; //ending the character array
>>     return newStr;
>> }
> 
> Won't this introduce a read-past-end-of-string in case the last character of 
> the string is a quote ?
> 
> As far as I understand that would make
> newStr[n] = '\0'
> and tmpstr will be increased twice before it's reevaluated, effectively moving 
> it past the final '\0':
> Once in
> newStr[n] = *(++tmpstr);
> and a second time in
> ++tmpstr;

Good point, though it’s also true of the original. I considered a for-loop instead, I’ll revisit that as less ugly than checking tmpstr twice.

Regards,
John Ralls