[freetds] some small patches...

Thompson, Bill D (London) ThompBil at exchange.uk.ml.com
Tue Apr 15 10:54:57 EDT 2003


Hi James,


No, free(NULL) does not cause me problems, I was just being careful.

> This could be simplified to:

> void 
> tds_free_context(TDSCONTEXT *context)
> {
>         tds_free_locale(context->locale);
>         TDS_ZERO_FREE(context);
> }

Well no... if  context is NULL, you'll segment on referencing
context->locale, so you need at least to check whether context is NULL or
not...

so I guess you could simplify to....

void tds_free_context(TDSCONTEXT *context)
{
    if (context) {
        tds_free_locale(context->locale);
        context->locale = NULL;
        TDS_ZERO_FREE(context);
    }    
}

Incidentally I added "context->locale = NULL;" as I had to do the same with
dbexit()...
   
if (g_dblib_ctx.tds_ctx) {
     tds_free_context(g_dblib_ctx.tds_ctx);
     g_dblib_ctx.tds_ctx = NULL;
}

Although tds_free_context freed the storage pointed to by *context ,
g_dblib_ctx.tds_ctx was still set, as the pointer had been passed by value,
not reference...
So I did the same thing when tds_free_context called tds_free_locale. Not
strictly necessary of course...

Oh, thanks for applying these by the way,

Bill





> -----Original Message-----
> From:	Lowden, James K [SMTP:LowdenJK at bernstein.com]
> Sent:	15 April 2003 15:41
> To:	'FreeTDS Development Group'
> Subject:	RE: [freetds] some small patches...
> 
> > From: Thompson, Bill D (London) [mailto:ThompBil at exchange.uk.ml.com]
> > Sent: April 15, 2003 8:38 AM
> >
> > As I am about to go into production with freetds in the next 
> > few weeks, I
> > don't want to take the latest snapshot with all the charset 
> > stuff
> 
> Good news, bad news, eh?  
> 
> I'm applying these, but I have a couple of questions.  
> 
> > +    case SYBDATETIME:
> > +        memset(varaddr, '\0', 8);
> > +        break;
> 
> Fine.  
> 
> > void
> > dbexit()
> ...
> Fine.  
> 
> > void tds_free_context(TDSCONTEXT *context)
> > {
> >     if (context && context->locale) {
> 
> tds_free_locale() returns immediately if its argument is NULL.  No need to
> check it here.  IMO the rule should be that the tds_free* functions can be
> passed null pointers harmlessly.  
> 
> >         tds_free_locale(context->locale);
> >         context->locale = NULL;
> >     }
> >     if (context)
> >         TDS_ZERO_FREE(context);
> > }
> 
> Does "free(NULL)" cause a problem with your libraries?  It's not supposed
> to; you should be able to free a null pointer without penalty.  If that
> *is*
> a problem, we should change TDS_ZERO_FREE() to avoid it, add a note to our
> CodingStyle or something, to let people (like me) know they can't rely on
> it.  
> 
> This could be simplified to:
> 
> void 
> tds_free_context(TDSCONTEXT *context)
> {
>         tds_free_locale(context->locale);
>         TDS_ZERO_FREE(context);
> }
> 
> Does that work for you?   
> 
> > void tds_free_locale(TDSLOCALE *locale)
> ...
> Already dealt with, per your prior message.  
> 
> Regards, 
> 
> --jkl
> 
> 
> The information contained in this transmission may contain privileged and
> confidential information and is intended only for the use of the person(s)
> named above.  If you are not the intended recipient, or an employee or
> agent
> responsible for delivering this message to the intended recipient, any
> review, dissemination, distribution or duplication of this communication
> is
> strictly prohibited. If you are not the intended recipient, please contact
> the sender immediately by reply e-mail and destroy all copies of the
> original message. Please note that we do not accept account orders and/or
> instructions by e-mail, and therefore will not be responsible for carrying
> out such orders and/or instructions.
> 
> 
> _______________________________________________
> FreeTDS mailing list
> FreeTDS at lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds



More information about the FreeTDS mailing list