Henrik Sjoberg wrote: >>>>>Don't bother. free(NULL) is perfectly fine. I would, on the >>>>>other hand, always NULL out a pointer that I've freed, unless >>>>>it is about to go out of scope. I.e.: >>>>> >>>>> free(p_description->extended_event.p_text_char); >>>>> p_description->extended_event.p_text_char = NULL; >>>>> >>>>>and thus avoid memory leaks or double-frees. Same elsewhere. >>>>> >>>>> >>>>> >>>>> >>>This is mainly in descriptor.c, which is not a rewrite, but a >>>restructuring. However, I could put in some work here too. >>>The entire memory handling would actually benefit from a review. >>> >>> >>> >>> >>I would suggest to have a functional change applied, then cleanups/other >>optimizations, rather than one single patch checked in. Would be easier >>for everybody. >> >> >> >>>I would too ;) Also in descriptor.c. I wanted to change as few things as >>>possible when I had no chance of testing it. >>> >>> >>> >>> >>> >>What i would say is, cosmetic changes should be a different patch rather >>than a functional patch. >>But sometimes that cannot be avoided, but generally we should go that >>way i think. >> >> >> > >So how should we do for now? Should we commit the changes as is now and I >will come up with a cosmetic patch after that? Or should I change my patch >according to the comments from Philip? > > > IMHO, If it is functional enough for people it should go in. The cosmetic fixes should go in as another patch probably. That would be a bit easier to handle. Manu
