If you don’t want to repeat, you can do it like this:
int Initialize(MyStruct *pMyStruct)
{
int iResult = 0;
if (pMyStruct != 0)
{
memset(pMyStruct, 0, sizeof(MyStruct));
pMyStruct->a = malloc(1000);
pMyStruct->b = malloc(1000);
pMyStruct->c = malloc(1000);
iResult = (pMyStruct->a != NULL) && (pMyStruct->b != NULL)
&& (pMyStruct->c != NULL);
if (iResult == 0)
{
Release(pMyStruct);
}
}
return iResult;
};
No gotos, one single return but potentially useless malloc’s. There is no
perfect solution but I prefer to do as shown above, unless the allocation of
b is dependent on the allocation of a (mapping for example).
In C++ it’s much better since you do:
MyStruct s;
// code
return;// resources get freed on return
If you don’t want to waste the stack
auto_ptr pS;
// code
return; // idem
The first language I learnt is BASIC on an Amstrad CPC 6128. I can tell you
it taught me to hate gotos.
–
EA
> -----Original Message-----
> From: xxxxx@lists.osr.com [mailto:bounce-287250-
> xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
> Sent: vendredi 18 mai 2007 22:49
> To: Windows System Software Devs Interest List
> Subject: RE: [ntdev] RES: Strange problem with VC++ compiler because of
> goto
>
> My approach is very similar except of multiple returns. Why to
> unnecessarily repeat the same code?
>
> BOOLEAN Initialize(MyStruct *pMyStruct) {
> BOOLEAN InitReady = FALSE;
>
> > if (pMyStruct != NULL) {
> > memset(pMyStruct, 0, sizeof(MyStruct));
> >
> > if ((pMyStruct->a = malloc(1000)) == NULL) {
> goto Done;
> > }
> > if ((pMyStruct->b = malloc(1000)) == NULL) {
> > goto Done;
> > }
> > if ((pMyStruct->c = malloc(1000)) == NULL) {
> goto Done;
> > }
> InitReady = TRUE;
> > }
> Done:
> if (!InitReady) {
> Release(pMyStruct);
> }
> > return(InitReady);
> > }
> >
> This is generic approach not related to allocations only. In such a
> simple case I’d use more structured form of goto:
>
> BOOLEAN Initialize(MyStruct *pMyStruct) {
> BOOLEAN InitReady = FALSE;
>
> do {
> > if (pMyStruct == NULL) {
> break;
> }
> > memset(pMyStruct, 0, sizeof(MyStruct));
> >
> > if ((pMyStruct->a = malloc(1000)) == NULL) {
> break;
> > }
> > if ((pMyStruct->b = malloc(1000)) == NULL) {
> > break;
> > }
> > if ((pMyStruct->c = malloc(1000)) == NULL) {
> break;
> > }
> InitReady = TRUE;
> > } while (FALSE);
> >
> if (!InitReady) {
> Release(pMyStruct);
> }
> > return(InitReady);
> > }
> >
> Best regards,
>
> Michal Vodicka
> UPEK, Inc.
> [xxxxx@upek.com, http://www.upek.com]
>
>
> > ----------
> > From: xxxxx@lists.osr.com[SMTP:bounce-287247-
> xxxxx@lists.osr.com] on behalf of Edouard A.[SMTP:xxxxx@fausse.info]
> > Reply To: Windows System Software Devs Interest List
> > Sent: Friday, May 18, 2007 10:15 PM
> > To: Windows System Software Devs Interest List
> > Subject: RE: [ntdev] RES: Strange problem with VC++ compiler because
> of goto
> >
> > If I have to do pure C (fortunately this is rare… :p), I like to do
> it
> > this way:
> >
> > #include <malloc.h>
> > #include <memory.h>
> >
> > struct _MyStruct
> > {
> >
> > void *a;
> > void *b;
> > void *c;
> >
> > };
> >
> > typedef struct _MyStruct MyStruct;
> >
> > int Initialize(MyStruct *pMyStruct);
> > void Release(MyStruct *pMyStruct);
> >
> > int Initialize(MyStruct *pMyStruct)
> > {
> >
> > if (pMyStruct != 0)
> > {
> >
> > memset(pMyStruct, 0, sizeof(MyStruct));
> >
> > pMyStruct->a = malloc(1000);
> > if (!pMyStruct->a)
> > {
> > Release(pMyStruct);
> > return 0;
> > }
> >
> > pMyStruct->b = malloc(1000);
> > if (!pMyStruct->b)
> > {
> > Release(pMyStruct);
> > return 0;
> > }
> >
> > pMyStruct->c = malloc(1000);
> > if (!pMyStruct->c)
> > {
> > Release(pMyStruct);
> > return 0;
> > }
> > }
> >
> > return 1;
> >
> > };
> >
> > void Release(MyStruct *pMyStruct)
> > {
> > if (pMyStruct)
> > {
> > if (pMyStruct->a != 0)
> > {
> > free(pMyStruct->a);
> > }
> >
> > if (pMyStruct->b != 0)
> > {
> > free(pMyStruct->b);
> > }
> >
> > if (pMyStruct->c != 0)
> > {
> > free(pMyStruct->c);
> > }
> >
> > memset(pMyStruct, 0, sizeof(MyStruct));
> >
> > }
> > };
> >
> > int main(int argc, char **argv)
> > {
> >
> > MyStruct s;
> >
> > Initialize(&s);
> >
> > Release(&s);
> >
> > }
> >
> > I always encapsulate my resources allocations inside a structure, if
> I need
> > to add more I only need to modify two functions. You can of course
> factor
> > further since we have here several malloc(1000) which should in fact
> emanate
> > from another Initialize.
> >
> > I have no goto, but I have several returns. In this case however it
> is hard
> > to miss the “release” before return because there is only one call to
> > release all. I try very hard to have functions as small as possible,
> big
> > functions => more bugs. This increases the number of functions and
> the
> > initial amount of work but, to my experience, reduces the amount of
> bugs.
> >
> > The code has to respect some sort of symmetry and harmony. When your
> code is
> > symmetric and harmonic, your brain is going to repeat intuitively the
> > patterns it sees and this reduces the occurrence of bugs.
> >
> > When you start to get
> >
> > while(1)
> > {
> > if (blah)
> > {
> > If ()
> > {
> > Break;
> > }
> > Else
> > {
> > }
> > }
> > else
> > {
> > if ((p = malloc(1000)) != 0)
> > {
> > Return;
> > }
> > else
> > {
> > Continue;
> > }
> > Break;
> > }
> > }
> >
> > You WILL have memory leaks and you WILL have bugs. It’s not a problem
> of
> > break, continue, return whatever unholy keyword it’s a problem of
> structure,
> > symmetry and harmony.
> >
> > –
> >
> > Edouard A.
> >
> >
> > > -----Original Message-----
> > > From: xxxxx@lists.osr.com [mailto:bounce-287215-
> > > xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
> > > Sent: vendredi 18 mai 2007 19:27
> > > To: Windows System Software Devs Interest List
> > > Subject: RE: [ntdev] RES: Strange problem with VC++ compiler
> because of
> > > goto
> > >
> > > We use similar approach and the rule is simpler: one return per
> > > function. It naturally leads to what you describe. I expected hard
> > > discussion but surprisingly, coworkers agreed with no problem.
> Probably
> > > because everybody of them already encountered a problem with
> overlooked
> > > return. Other reason is we use traces which print function name +
> > > parameters on entry and return value on leave. The label before
> leave
> > > trace is then natural.
> > >
> > > Personally, I take returns in the middle of the function as an
> evil. It
> > > becomes apparent when somebody embeds return in the macro.
> Something
> > > like this:
> > >
> > > #define CATCH_ERROR(Function) if (!Function) return
> > >
> > > Best regards,
> > >
> > > Michal Vodicka
> > > UPEK, Inc.
> > > [xxxxx@upek.com, http://www.upek.com]
> > >
> > >
> > > > ----------
> > > > From: xxxxx@lists.osr.com[SMTP:bounce-287149-
> > > xxxxx@lists.osr.com] on behalf of
> > > xxxxx@ixos.de[SMTP:xxxxx@ixos.de]
> > > > Reply To: Windows System Software Devs Interest List
> > > > Sent: Friday, May 18, 2007 9:04 AM
> > > > To: Windows System Software Devs Interest List
> > > > Subject: RE:[ntdev] RES: Strange problem with VC++ compiler
> because
> > > of goto
> > > >
> > > > hi,
> > > >
> > > > the problem with multiple returns in a function (especially in
> larger
> > > ones) is often, that some resources which are acquired (most often
> > > later during a change of the code) in that function are not freed
> at
> > > exit of the function because of an early “return”.
> > > >
> > > > Therefore - after hard discussions - we have the rule in our
> larger
> > > projects that each larger function has to have a label - most often
> > > named "exitFunction: " or so - near the end of the function. And
> > > everybody has to use “goto exitFunction” to leave the function.
> After
> > > that label all resources must be freed. Of course you have to keep
> > > track there which resources are really acquired when you reach this
> > > part of the code.
> > > >
> > > > And I think that it is completely structured code although there
> are
> > > those “gotos” inside the code. We have this discussion about gotos
> > > again and again, but I discuss about coding guidelines since more
> than
> > > 35 years now. It is very hard for my colleagues to argue with me
> about
> > > that point 
> > > >
> > > > – Reinhard
> > > >
> > > >
> > > > —
> > > > Questions? First check the Kernel Driver FAQ at
> > > http://www.osronline.com/article.cfm?id=256
> > > >
> > > > To unsubscribe, visit the List Server section of OSR Online at
> > > http://www.osronline.com/page.cfm?name=ListServer
> > > >
> > >
> > > —
> > > Questions? First check the Kernel Driver FAQ at
> > > http://www.osronline.com/article.cfm?id=256
> > >
> > > To unsubscribe, visit the List Server section of OSR Online at
> > > http://www.osronline.com/page.cfm?name=ListServer
> >
> >
> > —
> > Questions? First check the Kernel Driver FAQ at
> http://www.osronline.com/article.cfm?id=256
> >
> > To unsubscribe, visit the List Server section of OSR Online at
> http://www.osronline.com/page.cfm?name=ListServer
> >
>
> —
> Questions? First check the Kernel Driver FAQ at
> http://www.osronline.com/article.cfm?id=256
>
> To unsubscribe, visit the List Server section of OSR Online at
> http://www.osronline.com/page.cfm?name=ListServer</memory.h></malloc.h>
?