bugGPSD - Bugs: bug #44025, Boilerplate code used for common...

 
 

bug #44025: Boilerplate code used for common string operations

Submitted by:  Zbigniew Chyla <zchyla_nokia>
Submitted on:  Fri 16 Jan 2015 05:09:14 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Eric S. Raymond <esr>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Fri 16 Jan 2015 05:13:32 PM UTC, comment #1:

My proposal is to add simple helper functions for common string operations. See below for details.

Patches from 0001-* to 0006-* introduce new string helpers (as macros):

  • str_starts_with(str, prefix) - checks if str starts with the specified prefix,
  • str_appendf(str, alloc_size, format, ...) - appends formatted output to specified string (with allocated size check),
  • str_vappendf(str, alloc_size, format, ...) - va_list variant of str_appendf,
  • str_rstrip_char(str, ch) - strips the last character from str if it's equal to ch

In order to verify, whether these changes don't break anything, just build the code before and after applying the patches, with:

  • gpsd_version set to string without “~dev” suffix,
  • “-DNDEBUG” in CFLAGS.

In both cases stripped binaries should be identical.

Patch 0007-* converts all macros to functions. The conversion is safe, as long as each argument used in a macro more than once is free of side effects. In order to verify this, I temporarily changed definitions of the macros, adding extra occurrence of each argument, which was used more than once (patch “strfuncs-verify-side-effects.diff”). The change didn't affect the generated binary (with settings like above), which means that there were no side effects in these arguments.

Patches 0008-* to 0010-* do change the generated code, but they are rather small. The code passes regression tests after each patch.

Zbigniew Chyla <zchyla_nokia>
Fri 16 Jan 2015 05:09:14 PM UTC, original submission:

Gpsd contains a lot of boilerplate code for commonly performed string operations, such as:

  • appending formatted string: (void)snprintf(msgbuf + strlen(msgbuf), sizeof(msgbuf) - strlen(msgbuf), "...", ...);
  • stripping single character at the end: if (reply[strlen(reply) - 1] == ',') reply[strlen(reply) - 1] = '\0';
  • checking whether the string begins with specified prefix: strncmp(session->gpsdata.dev.path, "blahblah", 8) != 0

The problem with boilerplate code is that:

  • it's hard to read,
  • it's error-prone (see bug #43953 for examples, I'll report more such problems with fixes soon).
Zbigniew Chyla <zchyla_nokia>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by esr (Updated the item)
  • -unavailable- added by zchyla_nokia (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 14 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 22 Feb 2015 05:03:10 PM UTCesrOpen/ClosedOpen=>Closed
    Wed 21 Jan 2015 06:01:28 PM UTCesrStatusNone=>Fixed
      Assigned toNone=>esr
    Fri 16 Jan 2015 05:11:44 PM UTCzchyla_nokiaAttached File-=>Added strfuncs-verify-side-effects.diff, #32853
    Fri 16 Jan 2015 05:11:31 PM UTCzchyla_nokiaAttached File-=>Added 0010-Simplify-by-using-str_rstrip_char.patch, #32852
    Fri 16 Jan 2015 05:11:22 PM UTCzchyla_nokiaAttached File-=>Added 0009-Simplify-by-using-str_appendf.patch, #32851
    Fri 16 Jan 2015 05:11:13 PM UTCzchyla_nokiaAttached File-=>Added 0008-Support-empty-strings-in-str_rstrip_char.patch, #32850
    Fri 16 Jan 2015 05:11:06 PM UTCzchyla_nokiaAttached File-=>Added 0007-Convert-macros-in-strfuncs.h-to-functions.patch, #32849
    Fri 16 Jan 2015 05:10:58 PM UTCzchyla_nokiaAttached File-=>Added 0006-Add-compiler.h-with-compiler-specific-macros.patch, #32848
    Fri 16 Jan 2015 05:10:42 PM UTCzchyla_nokiaAttached File-=>Added 0005-Add-str_rstrip_char-use-it-everywhere.patch, #32847
    Fri 16 Jan 2015 05:10:31 PM UTCzchyla_nokiaAttached File-=>Added 0004-Add-str_-v-appendf-use-it-everywhere.patch, #32846
    Fri 16 Jan 2015 05:10:22 PM UTCzchyla_nokiaAttached File-=>Added 0003-Use-str_starts_with-instead-of-local-macro.patch, #32845
    Fri 16 Jan 2015 05:10:13 PM UTCzchyla_nokiaAttached File-=>Added 0002-Add-str_starts_with-macro-use-it-instead-of-strncmp.patch, #32844
    Fri 16 Jan 2015 05:09:55 PM UTCzchyla_nokiaAttached File-=>Added 0001-Add-new-header-for-string-functions.patch, #32843

    Back to the top


    Powered by Savane 3.1-cleanup1