Bug 23282

Created attachment 92661 [details]
handle the error code

The above patch tries to be completely in align with the coding style.This change in example will prevent the crashing of code.

Marshel, that patch does not address the ambiguity created by backwards compatibility with glibc 2.0.6, it merely removes the backwards compatilibility (while retaining notes that it is compatible). You also omit the return value in your return statement.

I'd be happy with the backwards compatibility removed (and a note to that effect), given glibc 2.0 is now 15 years old. The code could then be simplified. At the time I encountered this issue, I wrote the following code, which would just error out in the case of truncation with old glibc. http://code.google.com/p/opkg/source/browse/trunk/libopkg/sprintf_alloc.c

 (In reply to comment #2)

> I'd be happy with the backwards compatibility removed (and a note to that
> effect), given glibc 2.0 is now 15 years old. The code could then be
> simplified. 

Agreed. I'd accept a patch along those lines. Does someone want to write one?

Created attachment 93121 [details]
handle error code 

Okay this time I have tried to address the backward compatibility issue.I have made the change in NOTES section of the Documentation.I have gone through the entire Documentation and could find only two places where it is mentioned.

Regarding the return value I couldn't figure out what to return :-) apart from NULL.

I have kept the code simple and also test it.Though the codes have been changed comments are not felt like they fit perfectly.

Guys, Thanks a ton for your reply.I have added another patch. Waiting for your response.

Graham,

Could you review?

Thanks,

Michael

I'm not able to properly look at it this week... Due to connectivity problems I'm stuck to typing this on my phone.

Just to remind you that the patch have not been reviewed. Are you back to Good Internet...?

Can Somebody review the patch...?

(In reply to comment #6)
> Graham,
> 
> Could you review?
> 
> Thanks,
> 
> Michael


Michael,

Can I expect some response...?

Marshal, I don't apreciate being constantly badgered to review your patch. *One* email, or *one* comment here would have sufficed.


>--- a/man3/printf.3
>+++ b/man3/printf.3
>@@ -892,7 +892,9 @@ and
> conforms to the C99 standard, that is, behaves as described above,
> since glibc version 2.1.
> Until glibc 2.0.6 they would return \-1
>-when the output was truncated.
>+when the output was truncated.In current implementation a negative value 
>+is returned if an output error is encountered(see Return Value).Hence
>+these functions are not compatible with glibc 2.0.6 and before.

I'm unfamiliar with man page formatting conventions, but a cursory look at the rest of this file indicates that new sentences should be on new lines. In any event, I don't think this text adds anything useful.

> .\" .SH HISTORY
> .\" UNIX V7 defines the three routines
> .\" .BR printf (),
>@@ -1024,7 +1026,7 @@ With the value:
> one might obtain "Sonntag, 3. Juli, 10:02".
> .PP
> To allocate a sufficiently large string and print into it
>-(code correct for both glibc 2.0 and glibc 2.1):
>+(code incompatible for glibc 2.0.6 and before):

How about
"If truncation occurs in glibc versions prior to 2.0.6, this is treated as an error instead of being handled gracefully"

> .nf
> 
> #include <stdio.h>
>@@ -1050,18 +1052,21 @@ make_message(const char *fmt, ...)
>         n = vsnprintf(p, size, fmt, ap);
>         va_end(ap);
> 
>+      /* Check error code */
>+      
>+      if (n < 0)
>+          return NULL ;
>+           

The indentation here is not consistent with the rest of the example.

>         /* If that worked, return the string. */
> 
>-        if (n > \-1 && n < size)
>+        if (n < size)
>             return p;
> 
>         /* Else try again with more space. */
> 
>-        if (n > \-1)    /* glibc 2.1 */
>+        else
>             size = n+1; /* precisely what is needed */
>-        else           /* glibc 2.0 */
>-            size *= 2;  /* twice the old size */
>-
>+        
>         if ((np = realloc (p, size)) == NULL) {
>             free(p);
>             return NULL;
>-- 
>1.7.0.4

@Graham: Thanks for the review Graham.

@Marshel: Thanks for the revised patch. I appreciate that you're keen to get your patch reviewed, but everyone's on private time here, so things can be slow. As such, it's best to space your pings out a bit. I'm inclined to agree with Graham's comment that the first piece of text that you added probably isn't needed. But, otherwise, if you can fix the rest of the patch up as per Graham's comments, I'm happy to apply it.

Created attachment 94071 [details]
handle error code: patch no.3

*One Comment* :-)

Sorry for the trouble and Thanks for your patience. I think indentation is right this time.

Please review and let me know about further changes needed.

acked-by: graham.gower@gmail.com

Applied.

Marshel, thanks for the fix to Graham's bug!