bugGPSD - Bugs: bug #44265, Problems with timespec_str()...

 
 

bug #44265: Problems with timespec_str() function

Submitted by:  Zbigniew Chyla <zchyla_nokia>
Submitted on:  Mon 16 Feb 2015 12:42:25 AM 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.

 

Mon 16 Feb 2015 03:11:42 AM UTC, comment #1:

I took all of these except #5. I see the point of it, but a semantically meaningless structure wrapper is too much ceremony for me.

Eric S. Raymond <esr>
Project AdministratorIn charge of this item.
Mon 16 Feb 2015 12:42:25 AM UTC, original submission:

I've found a few problems with timespec_str() function, recently added to "gpsutils.c":

  • the code added to "gpsd_json.c" during conversion to timespec_str() doesn't compile when TIMING_ENABLE and PPS_ENABLE are defined;
  • the function uses a pointer to non-const struct as its input argument;
  • the function accepts buffer size as signed int value (size_t should be used instead);
  • the function code doesn't match its description - it returns the result of snprintf() call, i.e. not necessarily the number of chars written to the buffer (if the buffer provided by the caller is too small for generated string);
  • all callers ignore value returned by the function, so it doesn't make sense to return anything.

Since the function needs output buffer of specific size (known at compilation time), it would be best to enforce it statically, instead of accepting buffer size as runtime argument. I suggest using dedicated type (struct with char[] field) for storing the result. This approach makes it impossible to call the function with incorrectly sized buffer.

I'll attach patches that fix these problems.

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 (Posted a comment)
  • -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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 22 Feb 2015 05:02:35 PM UTCesrOpen/ClosedOpen=>Closed
    Mon 16 Feb 2015 03:11:42 AM UTCesrStatusNone=>Fixed
      Assigned toNone=>esr
    Mon 16 Feb 2015 12:43:23 AM UTCzchyla_nokiaAttached File-=>Added 0005-Use-dedicated-struct-type-for-timespec_str-output.patch, #33093
    Mon 16 Feb 2015 12:43:11 AM UTCzchyla_nokiaAttached File-=>Added 0001-Fix-compilation-error.patch, #33089
      Attached File-=>Added 0002-Avoid-warning-pointer-to-volatile-struct.patch, #33090
      Attached File-=>Added 0003-timespec_str-add-const-to-input-argument.patch, #33091
      Attached File-=>Added 0004-Remove-unused-and-incorrect-retval-from-timespec_str.patch, #33092

    Back to the top


    Powered by Savane 3.1-cleanup1