Create and insert material

[Helper]

On 31/10/2016 at 04:36, xxxxxxxx wrote:

Hi Scott,

Thanks for the clear explanation.
It is more clear now.

Pim

[Helper]

On 31/10/2016 at 08:19, xxxxxxxx wrote:

Hi,

please hold on, I think, we need to clear some thing's up.

First of all: The initial error (reason why SearchMaterial() didn't work in scope of if), it is just a matter of "masking" a local variable by redefining it in the scope of the if, namely "mymat". To avoid such problems, please increase the warning level in your project.

I'm sorry, Scott, but your explanation for AutoAlloc is a bit off.
Of course one can (and should!) use AutoAlloc with objects and/or materials. We recommend to think about it's use whenever you allocate any memory. It really helps avoiding memory leaks. In the end, it's all about ownership. If you allocate something via AutoAlloc, the AutoAlloc instance owns the pointed object. If you then pass the ownership to Cinema 4D, you need to Release() the object from the AutoAlloc instance, and of course you must not use the AutoAlloc instance after the release.

So your code, Scott, correctly looks like so:

    AutoAlloc<BaseMaterial> mat(Mmaterial);
    if (!mat)
        return;
    mat->SetName("XXX");
    doc->InsertMaterial(mat.Release());  // Transferring ownership from AutoAlloc to C4D
    // GePrint(mat->GetName()); // After release the AutoAlloc instance can't be used anymore!
  
    // If that is really needed, then save the actual BaseMaterial pointer before:
    AutoAlloc<BaseMaterial> mat2(Mmaterial);
    if (!mat2)
        return;
    mat2->SetName("XXX");
    BaseMaterial* bm2 = mat.Release(); // Getting ownership from AutoAlloc instance
    doc->InsertMaterial(bm2);  // Transferring ownership to C4D
    GePrint(bm2->GetName());

The point here should be obvious: After the AutoAlloc (and before the Release()) you can leave the scope/function for example because of an error at any time without having to worry about freeing the material.
Also be aware, that you still need to check for nullptrs, when using AutoAlloc.

In the cinema4dsdk examples, there are several examples of correct AutoAlloc usage.
And the latest documentation also contains some more information on AutoAlloc.

[Helper]

On 31/10/2016 at 09:58, xxxxxxxx wrote:

It's a good thing that this thread came up.
Because for roughly the past 10 years or so. None of us here in the forums (including the support guys) have been using AutoAlloc for basic material and object creation. And neither did the official SDK examples. We all used Alloc without manually freeing it.
We were told that C4D handles the memory for us in these cases.
Now I see it being used in the new docs.

AFAIK. None of us have had any problems with memory leaks using Alloc and not manually deleting the memory ourselves (letting C4D handle it).
And when using the debugger. There are no memory leaks shown when using Alloc this way.

When storing and deleting objects in lists I can see how AutoAlloc could help to prevent leaks.
But for basic creation and deletion code? This seems like extreme overkill to me (paranoia).
But I guess Maxon has to push safety. Because you don't know in what context people will be creating & deleting things.

-ScottA

[Helper]

On 31/10/2016 at 10:50, xxxxxxxx wrote:

Hi Scott,

C4D can only the memory deallocation, when ownership of an object has been transferred to it (e.g. in above code via InsertMaterial()). Up to that point you are responsible on your own.
Take this example very simple example, pseudo code, but you will get the idea. Notice how you need to carefully free the allocated materials on every error exit:

BaseMaterial* mat = BaseMaterial::Alloc(Mmaterial);
if (!mat)
  return;
BaseMaterial* mat2 = BaseMaterial::Alloc(Mmaterial);
if (!mat2)
{
  BaseMaterial::Free(mat);
  return;
}
if (other error)
{
  BaseMaterial::Free(mat);
  BaseMaterial::Free(mat2);
  return;
}
mat->SetName("my own material");
  
// doing some fancy stuff with mat and mat2
// ...
  
// and successfully finish
doc->InsertMaterial(mat);
BaseMaterial::Free(mat2);
return;

Instead with AutoAlloc:

BaseMaterial* mat = BaseMaterial::Alloc(Mmaterial);
BaseMaterial* mat2 = BaseMaterial::Alloc(Mmaterial);
if (!mat || !mat2)
  return;
if (other error)
  return;
mat->SetName("my own material");
// doing some fancy stuff with mat and mat2
// ...
  
// and successfully finish
doc->InsertMaterial(mat.Release());
return;

Not only does it need less code lines, but the only point, where you need to take care and think about is when transferring the ownership to C4D on inserting the one material into the document.

[Helper]

On 31/10/2016 at 11:46, xxxxxxxx wrote:

I will try to use AutoAlloc more often in my code in the future.
Even though I've gotten good results from Alloc. I can see the benefits of using it.
It helps to write more defensive code easier.

-ScottA

[Helper]

On 01/11/2016 at 06:10, xxxxxxxx wrote:

Sorry, I do not fully understand.
Especially the meaning of ownership and the impact on my plugin programming code.

What I want to do: check whether a material already exists.
If not, create the material and pass on the link of the material.
If the material already exists, pass on the link of the material.

Scott's working example.

    //Create the empty material pointer
    BaseMaterial *mymat = NULL;
  
    //Check if the material already exists
    mymat = doc->SearchMaterial("my matl");
    if (!mymat)
    {
        mymat = BaseMaterial::Alloc(Mmaterial);
        mymat->SetName("my matl");
  
        doc->InsertMaterial(mymat);
        mymat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1,0,0), DESCFLAGS_SET_0);
    }

Using following code with AutoAlloc does not work.
Adding .Release()makes things even worse.

//Create the empty material pointer
BaseMaterial *mymat = NULL;
  
//Check if the material already exists
mymat = doc->SearchMaterial("my mat");
  if (!mymat)
 {
        AutoAlloc<BaseMaterial> mymat(Mmaterial);
        if (!mymat) return;
        mymat->SetName("my mat");
        doc->InsertMaterial(mymat());
        //doc->InsertMaterial(mymat.Release());
        mymat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1,1,0), DESCFLAGS_SET_0);
}
  
Could you please give some more explanation.
  

[Helper]

On 01/11/2016 at 08:49, xxxxxxxx wrote:

If you don't understand ownership. Then AutoAlloc is going to be a bit confusing in this usage.
Because in this usage it involves passing ownership around a little bit.

BaseMaterial *mymat = nullptr; <--- you own this pointer. So you must use free on it yourself to kill it without memory leaks
doc->InsertMaterial(mymat);    <--- C4D owns it. You don't need to worry about freeing it yourself anymore.

You need to pass (handoff) what AutoAlloc created to a pointer. Then kill the AutoAlloc yourself using release(). Because nothing has been passed to C4D yet.
Like this:

    //Create the empty material pointer  
  BaseMaterial *mymat = NULL;  
  
  //Check if the material already exists  
  mymat = doc->SearchMaterial("my mat");  
  if (!mymat)  
  {  
      AutoAlloc<BaseMaterial> aam(Mmaterial); //Creates an instance of the Mmaterial class  
      if (!aam) return false;  
  
      mymat = aam.Release();   //<---Passes the instance created in aam to the pointer "mymat". Then kills aam  
  
      //Now you can use mymat from here on out...  
      mymat->SetName("my mat");  
      doc->InsertMaterial(mymat);  
      mymat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1, 1, 0), DESCFLAGS_SET_0);  
  }

When you compare this AutoAlloc version to the Alloc version. You might be inclined to say that the AutoAlloc version is more complex. So why would I use it instead of the simpler Alloc version?
The answer is. You don't have to. The simple Alloc version works just fine.

But...
The Alloc code we're using is cheating here. It never checks if Alloc fails. So it's technically not correct code. It's considered a bad practice.
If you did this in an interview you would probably not get the job.
And if you put in the code that checks for that(like in Andreas's example) the code becomes longer than the AutoAlloc code version.
But...but....In reality. Alloc rarely ever fails. And if it does fail C4D will crash. And I personally want that to happen.
I want C4D to crash upon a failed allocation. I don't want it to continue on a do something funky and not know where it's gone wrong.
But this is not always the case. In some cases outside of the C4D world your boss might not want the program to crash hard. He might want to fail softly instead.

In summation
The Alloc code I posted will work just fine. And hundreds of people have used it for many years.
But it's technically not correct code unless to add in a check for a failed Alloc.
If you decide to use the Alloc version. Don't throw away the AutoAlloc completely. Because where it's most useful is when you are handling multiple instances of materials. Swapping them, storing & retrieving them from arrays, etc.. That's where AutoAlloc can really help keep you safe from accidental memory leaks, and cut down on the number of error checks needed to be written manually.
It might be a good thing to force yourself to use it even for simple insertion where it's a bit of overkill. Just to get comfortable to using it.

-ScottA

[Helper]

On 01/11/2016 at 11:40, xxxxxxxx wrote:

I guess, I have to add MAXON's point of view here.

While during development you might want a failed allocation to end up in a crash (which I (just my personal opinion) think, is pretty bad practice already), in your final plugin, this should not and must not happen. User will loose work and precious time. This is an absolute no go!
Here at MAXON such code is not considered bad practice, but plain wrong.

Next:
"Allocations rarely fail" is simply not true. They might not fail in your test environment, but in real world users tend to work with huge scenes with lots and lots of high poly objects and high res textures and even on today's machines it is very well possible to run out of memory. Believe me, you don't want to be the reason for a user to bite into his/her table.
Every allocation needs to be checked! Full stop.
In snippets in this forum (and also the ones I have below) these checks might be omitted to focus on some issues and to avoid too much code, but in real world code each and every allocation needs to be checked. And there's nothing funky going to happen afterwards, but your code will determine how to take care of such error condition.

So please never do something like this. Rather use a DebugStop() (or even a CriticalStop()) to be informed about an allocation problem during development. But this error branch has to work without crash, when you deem your development done.
So do something like this:

BaseMaterial *mymat = BaseMaterial::Alloc(Mmaterial);
if (myMat == nullptr)
{
    DebugStop(); // TODO: handle error gracefully
    return;
}

Next: Ownership:
I don't want to be picky, but I think, we should get the details right here.

First some terms:

A pointer variable points to a certain memory address (nothing more and nothing less).
nullptr (or NULL in older versions of the SDK is a value of an invalid memory address (not necessarily zero!).

Please take a look at the terms Heap and Stack (or Call Stack) yourself, they are somewhat important in this context.
For now very simplified:
The stack is not only containing the order of calls to your functions (and their parameters) but also local variables. The stack grows and shrinks dynamically during a programs execution.
In this context we care only about the local variables:

void myFunc()
{
    Int32 myVar;
    // ...
}

So when this function is called (besides other things) there's made some room on the stack for myVar. And when the function is left, this space will automatically be "freed" again.

So following line from Scott:

BaseMaterial *mymat = nullptr; // <--- you own this pointer. So you must use free on it yourself to kill it without memory leaks

Well, technically not quite, the pointer resides on the stack. It is just a local variable. The concept of ownership is not relevant at this point.

Heap is an area of memory (actually an address space to be precise) a process can utilize for memory allocation. Of course this memory is not endlessly large. So whenever memory gets allocated, the underlying memory management reserves a section of the heap and returns a pointer to this section. At this point the concept of ownership comes into play. The allocating code gets owner of this section of memory and is responsible for deallocating/freeing this memory when it is no longer needed. So it is absolutely mandatory to not loose this pointer, otherwise you have a memory leak and this section is no longer usable for any other part of the program.

Example of a memory leak, kids don't do this at home:

void memoryLeak()
{
    // Memory is allocated and you are automatically owner of the allocated memory.
    // The resulting pointer is stored in a local variable on the stack
    BaseMaterial* mat = nulltpr; // just a pointer variable on the stack, no ownership involved, yet
    mat = BaseMaterial::Alloc(Mmaterial); // now, memory got allocated and you own the memory that the pointer points to
    // Now, leaving the function, the local pointer will be lost.
    // Result, you have no more reference to that part of memory and are not able to deallocate it later on.
    return;
}

Possible solutions:
- You want to stay owner of this memory, then store the pointer in a somewhat permanent place, for example a member variable of your class. But be aware, you are responsible for freeing this memory again, for example on destruction of your class instance.
- You pass the ownership to somebody else, who will take care of deallocation, when the memory is no longer needed.

The later case is usually needed, when an object/material/whatever gets inserted into a document. Obviously C4D needs to be the owner of such memory. Only C4D knows, when the user decides to delete the material. So by handing over the ownership of a piece of memory, you transfer the responsibility and permission to deallocate a piece of memory to another part of the program, in this case C4D's core.

doc->InsertMaterial(mymat);  // <--- C4D owns it. You don't need to worry about freeing it yourself anymore.

I'd rather say the ownership gets transferred to C4D and you do not only need to worry, but also are not allowed to free this piece of memory yourself. C4D relies on this memory to stay intact until it decides on its own that it is no longer needed.

Ok, many words, but I hope, I was able to explain the concept of ownership of allocated memory.

Now, lets look at AutoAlloc.
Basically, AutoAlloc simply wraps the usual Alloc() call and stores the resulting pointer in a member variable. AutoAlloc owns the allocated object. In its destructor AutoAlloc will then use the usual Free() to deallocate the memory (it is allowed to as it owns the allocated object).

void noMemoryLeak()
{
    AutoAlloc<BaseMaterial> mat;
    return;
}

Now, there is no memory leak anymore. The AutoAlloc instance gets created on the stack. It allocates the BaseMaterial, stores the pointer internally and when the function is left, the AutoAlloc instance gets removed from the stack, its destructor gets called and the BaseMaterial object gets deallocated. Nice!

The only "magic" involved with AutoAlloc is, that it overloads the dereference operator ("->"), so that -> being used with the AutoAlloc object will result in dereferencing the internally stored pointer, which is basically the pointer to your BaseMaterial in above example.

Another bad example:

void myPersonalCrash(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat(Mmaterial);
    doc->InsertMaterial(mat);
    return;
}

This will sooner or later lead to a crash. And I guess by now, you also know why. The material gets allocated and the AutoAlloc instance owns the pointed material. By inserting the material into the document, we tell C4D to take the ownership of that material. Then the function is left, the AutoAlloc instance gets destroyed, its destructor deallocates the material. And as soon as C4D tries to access the material, being of the opinion that it's owning this memory, it will crash (either by accessing already freed memory or by trying to free the memory a second time).

Solution:

void myPersonalCrash(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat(Mmaterial);
    doc->InsertMaterial(mat.Release());
    return;
}

With Release() we tell the AutoAlloc instance to no longer care/be responsible/own the pointed memory. It will do nothing else, than setting its internal member variable to nullptr. So after calling Release() the ownership of the pointed object gets transferred to the caller of Release().

    doc->InsertMaterial(mat.Release());

So in this line, mat.Release() transfers the ownership to you and you pass it directly on to C4D via InsertMaterial().

Now, one more pitfall:

void ohNoesAnotherCrash(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat(Mmaterial);
    doc->InsertMaterial(mat.Release());
    mat->SetName("BOOM");
    return;
}

This code is crashing, because during Release() the AutoAlloc instance sets its internal pointer member variable to nullptr. And by mat->SetName() this internal nullptr will be dereferenced. So if you need to work with an object/piece of memory formerly owned by an AutoAlloc, but by now no longer owned by the AutoAlloc due to calling Release(), then you need to store the returned pointer and use that one instead.

So either:

void yeahNoMoreCrash(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat(Mmaterial);
    BaseMaterial* bm = mat.Release();
    doc->InsertMaterial(bm);
    bm->SetName("silence");
    return;
}

Or:

void stillNoCrash(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat(Mmaterial);
    mat->SetName("silence");
    doc->InsertMaterial(mat.Release());
    return;
}

One more thing on the benefits of AutoAlloc:
Usually you will have way more error exits in your code, than successful ones. And you usually transfer the ownership of an object/material only in the case of success.
Using normal Alloc, you have to take care not to miss any of the allocations in all your error exits, otherwise: memory leak. And yes, in the success case, when transferring the ownership, you don't need to do anything.
Now, with AutoAlloc it's exactly the other way round. You don't need to pay attention to all your error exits anymore, but instead you will need to pay attention to the one place, where you transfer the ownership in the successful case.
So AutoAlloc not only has the potential of reducing the lines of code you need to write, but it also reduces the amount of locations in your code, where you need to be careful and pay attention.

It's actually just a matter of using it a few times and you will get used to it and won't want to miss it anymore. I promise. And by the way, there's also AutoFree... but I hope with the above explanations AutoFree is no longer a secret anymore.

Ok, now I'll shut up. Sorry for the long post, but I really felt we needed to clarify some things. And I really hope, I was able to. If not, if there are any questions left on this topic, please ask. Memory allocation, ownership, leaks is one of the most important topics in development and we should leave no questions unanswered here. Of course you could also use DuckDuckGo to learn more about the concepts and you will probably find somebody who is way better in explaining this stuff than I am.

Edit 20:00 CET: Fixed wikipedia links for stack and callstack

[Helper]

On 01/11/2016 at 14:01, xxxxxxxx wrote:

Nice meaty post Andreas.
I like this kind of stuff. It's very helpful to me. But we might have confused the heck out of poor Pim.!LOL[URL-REMOVED]
My bad. I meant to say that the allocation needs to be freed.

It must take a really huge scene to make Alloc fail. Because if I use Alloc correctly (where it doesn't crash) then it simply never fails. Ever.
I've often wondered in what context does an Alloc actually fail. Because it has never happened to me, or anyone I know of. So I always felt kind of silly error checking it.
I guess we aren't pushing it it hard enough.

-ScottA

---

[URL-REMOVED] @maxon: This section contained a non-resolving link which has been removed.

[Helper]

On 01/11/2016 at 14:41, xxxxxxxx wrote:

Originally posted by xxxxxxxx

Nice meaty post Andreas. I like this kind of stuff. It's very helpful to me. But we might have confused the heck out of poor Pim.!LOL[URL-REMOVED]My bad. I meant to say that the allocation needs to be freed.It
must take a really huge scene to make Alloc fail. Because if I use Alloc
correctly (where it doesn't crash) then it simply never fails. Ever.I've
often wondered in what context does an Alloc actually fail. Because it
has never happened to me, or anyone I know of. So I
always felt kind of silly error checking it.I guess we aren't pushing it it hard enough.-ScottA

Even simple scenes can make your allocation fail if your OS is running out of disc space while performing other tasks that either need some memory too and write a lot of data (think of video conversion), or because the OS is downloading some huge OS updates in parallel (think of the big Win 10 updates).

We see that from time to time in crash logs and it is an issue even on 64 bit systems as disc space can be limited - esp. on SSD drives.

Best regards,

Wilfried

---

[URL-REMOVED] @maxon: This section contained a non-resolving link which has been removed.

[Helper]

On 01/11/2016 at 15:02, xxxxxxxx wrote:

Thanks Wilfried,
Never ran into this one either. But good to know.

-ScottA

[Helper]

On 02/11/2016 at 01:49, xxxxxxxx wrote:

Yes, I am confused, but everything is very good information, thanks Scott and Andreas.
I will need some time to digest all the information.

What I learned so far:
- what I did wrong initially ( "masking" a local variable by redefining it in the scope of the if)
- use autoalloc to create new material

Here is the code (based on Scott's AutoAlloc example) I am using now:

	//Create the empty material pointer
    BaseMaterial *mymat = NULL;
  
    //Check if the material already exists
    mymat = doc->SearchMaterial("my mat");
    if (!mymat)
    {
        AutoAlloc<BaseMaterial> aam(Mmaterial); //Creates an instance of the Mmaterial class
        if (!aam) return;
  
        mymat = aam.Release();   //<---Passes the instance created in aam to the pointer "mymat". Then kills aam
  
        //Now you can use mymat from here on out...
        mymat->SetName("my mat");
        doc->InsertMaterial(mymat);
        mymat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1, 1, 0), DESCFLAGS_SET_0);
    }

[Helper]

On 02/11/2016 at 02:43, xxxxxxxx wrote:

Well, you don't have to use AutoAlloc, it's a recommendation to use it. And the way the code snippet you posted is written, it somehow makes AutoAlloc useless and kind of defeats the benefits of AutoAlloc. Basically calling AutoAlloc and directly afterwards Release() is the same as just using the normal Alloc() directly, just a bit obfuscated.

So, in order to reduce your confusion, let's write your example without AutoAlloc (after all you want to continue to use the pointer to the material later on, right?) :

    //Create the empty material pointer
    BaseMaterial* mymat = nullptr;
  
    //Check if the material already exists
    mymat = doc->SearchMaterial("my mat");
    if (!mymat)
    {
        myMat = BaseMaterial::Alloc(Mmaterial);
        if (!myMat)
            return;
  
        myMat->SetName("my mat");
        myMat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1, 1, 0), DESCFLAGS_SET_0);
        doc->InsertMaterial(myMat);
    }
    // Here myMat is a valid pointer to your material
    // But remember, the owner of the material pointed to by myMat is C4D at this point
    // ...

And, please, if your confusion holds on after some time of digestion, come back here and ask. As I said, I'd really like to get this straight.

[Helper]

On 02/11/2016 at 07:23, xxxxxxxx wrote:

That confuses me again.
You say "Well, you don't have to use AutoAlloc, it's a recommendation to use it.",
but I read "Here at MAXON such code is not considered bad practice, but plain wrong."

So, what is the benefit of autoalloc and when to use it?

I use the mymat pointer to add the mat to an object using the texture tag.

    //#assign material to object (texture tag)
    BaseTag *tag = sphereobj->MakeTag(Ttexture);    
    tag->SetParameter(TEXTURETAG_MATERIAL, mymat, DESCFLAGS_SET_0);
  

In this / your example, do I have to free the material at some point?
Or is cinema the owner because it is inserted in the doc?

Do I need to use the autoalloc if I wish to change the mat after inserting it into the document?

Being on the subject.
Here my code to insert an object using alloc.
Can the code be improved using autoalloc?

//#print "Create Default Sphere."
BaseObject* sphereobj = BaseObject::Alloc(Osphere);		//Create new sphere
sphereobj->SetParameter(PRIM_SPHERE_RAD, citysphereradius, DESCFLAGS_SET_0);	// set sphere label radius			
sphereobj->SetRelPos(Vector(x,y,z));						// Set position of sphere
sphereobj->SetName(country + " - " + city);
sphereobj->SetParameter(ID_LAYER_LINK, citylayer, DESCFLAGS_SET_0);  
doc->InsertObject(sphereobj, parentobject, NULL);      //Add it to the temp document 

[Helper]

On 02/11/2016 at 07:41, xxxxxxxx wrote:

Originally posted by xxxxxxxx

the way the code snippet you posted is written, it somehow makes AutoAlloc useless and kind of defeats the benefits of AutoAlloc.

But you're using the exact same code Andreas. That's where my code came from. By copying yours.
This is why I called it "overkill" earlier. For simple creating and insertion tasks.
Now you're starting to confuse me too. !Wacko[URL-REMOVED]

Originally posted by xxxxxxxx

And, please, if your confusion holds on after some time of digestion, come back here and ask. As I said, I'd really like to get this straight.

It would help me out if you posted an example using AutoAlloc that looks for an existing material. And creates a new one if it does not exist.
Only this time. Don't use this kind of code. Because you're saying it's useless. Even though it's copied straight from your own code examples:

    AutoAlloc<BaseMaterial> mat(Mmaterial);  
  if(!mat) return false;  
  BaseMaterial *bm = mat.Release();  
  doc->InsertMaterial(bm);  

Can you please post a version of this task that uses a better implementation of AutoAlloc where you don't consider it to be useless?

Thanks,
-ScottA

*Edit- This is another thing that is not being addressed clearly here and helping to add to the confusion.

BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);  
if(!mat) return false;   
  
//Is not the same thing as this  
  
BaseMaterial *mat = nullptr;  
if(condition) mat = BaseMaterial::Alloc(Mmaterial);  

When you set a pointer to nullptr like Pim is doing. You'll need to keep on checking it's existence every time you use it later on in the code. Because it's been allocated inside of the if() statement.
But if you create the pointer and allocate it all in the same line like in the docs. Then you only need to check it's existence once directly after it.
Most of the time support guys here have recommended to use: BaseMaterial *mat = nullptr;
Yet the docs mostly use the all-in-one version.
I can see this as also being a point of confusion for people not yet good with using the rules of scope.

---

[URL-REMOVED] @maxon: This section contained a non-resolving link which has been removed.

[Helper]

On 02/11/2016 at 09:44, xxxxxxxx wrote:

I'm terribly sorry for adding to the confusion.

Please, also keep in mind, that this discussion started with Pim's code, but you, Scott, made a very general statement about AutoAlloc. Pim's code was never an ideal candidate for AutoAlloc. But I had to correct your general statement about AutoAlloc, as it was simply wrong.

Originally posted by xxxxxxxx

You say "Well, you don't have to use AutoAlloc, it's a recommendation to use it.",
but I read "Here at MAXON such code is not considered bad practice, but plain wrong."

The "code is considered plain wrong" is all about not checking allocated memory. Regardless of using normal Alloc() or AutoAlloc, you need to check the result for nullptr.

AutoAlloc is just a tool to make your code less error prone and more readable in regards to memory allocations. And while we recommend it, you of course need to decide on a case by case basis, if it makes sense to use or if you gain any benefits from its use.

As explained before, AutoAlloc shifts the focus where you need to pay attention to. But if you have no error exits in between AutoAlloc and transferring the ownership via Release(), what would be the point?

But ok, a few more examples (some already posted above) :

Bool idealForAutoAlloc()
{
    AutoAlloc<BaseFile> file;
    if (!file)
        return false;
    // ... doing stuff...
    if (some error condition)
        return false;
    // ... doing stuff...    
    return true;
}

So this case is ideal, because you need to allocate the BaseFile in order to access a file, but regardless of success or error, you are not interested in keeping the BaseFile instance. So here, after AutoAlloc, your mind is completely free to do important work, you need not worry about the allocated memory at all.

Next one:

Bool goodUseOfAutoAlloc(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat;
    AutoAlloc<BaseMaterial> mat2;
    if (!mat || !mat2)
        return false;
    // ... doing stuff...
    if (some other error)
        return false;
    doc->InsertMaterial(mat.Release());
    return true;
}

I leave it up to you, to write the above snippet without AutoAlloc. But I think you can already sense, it will be quite a few more lines of code. And a few points where you need to pay attention to not create a memory leak.

Another one:

void probablyBetterWithStandardAlloc(BaseDocument* doc)
{
    BaseMaterial* mat = BaseMaterial::Alloc(Mmaterial);
    if (!mat)
        return;
    // doing stuff here, that can't fail...
    doc->InsertMaterial(mat);
    // ... doing more stuff, that for some reason needs to still work with the pointed material (although ownership got transferred already)
    return true;
}

This example is probably not well suited for AutoAlloc. There are no error cases, where you'd need to pay attention to not create a memory leak. The pointed material is to be used after the ownership got transferred, which would add an extra line of code in AutoAlloc case (using Release() to get hold of the actual BaseMaterial pointer). So there's little to no benefit in using AutoAlloc.

To sum this up:
- AutoAlloc can be a very nice tool...
    - ... to make memory allocations less error prone (memory leaks)
    - ... to have nicer and more readable code
- You still need to decide if this tool suits your needs in a given case
- Sometimes a small restructuring of your code might make it very well suited for AutoAlloc and you end with a nicer code structure

Perhaps another way to see it:
AutoAlloc lets you use memory allocations more or less like local variables.

Originally posted by xxxxxxxx

    //#assign material to object (texture tag)
    BaseTag *tag = sphereobj->MakeTag(Ttexture);    
    tag->SetParameter(TEXTURETAG_MATERIAL, mymat, DESCFLAGS_SET_0);

In this / your example, do I have to free the material at some point?
Or is cinema the owner because it is inserted in the doc?

In these very few lines? No, Cinema 4D does not get owner, just because you set a link pointing to a material in a tag. You'd need to explicitly insert the material.
Look at the docs for InsertMaterial() for example, there it is explicitly stated that the document will take the ownership of the material pointed to by the mat parameter. Basically whenever there are pointers involved, the docs should state, who's the owner or if ownership gets transferred in any way. If such comments are missing, please make us aware.

Originally posted by xxxxxxxx

Do I need to use the autoalloc if I wish to change the mat after inserting it into the document?

Should be clear by now, but you never have to use AutoAlloc. It's recommended to evaluate if it's suitable and use it where ever you can leverage its benefits.

Originally posted by xxxxxxxx

Here my code to insert an object using alloc.
Can the code be improved using autoalloc?

//#print "Create Default Sphere."
BaseObject* sphereobj = BaseObject::Alloc(Osphere);		//Create new sphere
sphereobj->SetParameter(PRIM_SPHERE_RAD, citysphereradius, DESCFLAGS_SET_0);	// set sphere label radius			
sphereobj->SetRelPos(Vector(x,y,z));						// Set position of sphere
sphereobj->SetName(country + " - " + city);
sphereobj->SetParameter(ID_LAYER_LINK, citylayer, DESCFLAGS_SET_0);  
doc->InsertObject(sphereobj, parentobject, NULL);      //Add it to the temp document 

Except for the missing nullptr check after the Alloc it's fine. But that check should really be added.

@Scott: I hope, my above examples help.

[Helper]

On 02/11/2016 at 10:07, xxxxxxxx wrote:

Thanks Andreas. I think I've got it. Thanks for your input.

I've probably only made things more confusing in this thread by asking questions on top of Pim's questions. So I'm going to exit this thread from here on out.
The only reason I even replied to his initial question was because he asked it during the weekend. Normally would not have replied during the week.

So I'm going to shut up and go away now.

-ScottA

[Helper]

On 02/11/2016 at 10:11, xxxxxxxx wrote:

Well, Scott, I think your post brought up a very crucial point. It's good we discussed it. And if I was able to shed some light onto this topic, than I'm glad.
That's not supposed to mean, open questions shouldn't be asked anymore. Quite the opposite.

[Helper]

On 02/11/2016 at 13:06, xxxxxxxx wrote:

My main confusion is with freeing memory.
I thought the main advantage if AutoAlloc is, that you need not to worry about freeing memory.
I know now - reading all the posts above - that you only need to worry about freeing memory if you are the owner.
If cinema is the owner, for example after an insert, you need not to worry about freeing memory.
So, if you are going to insert, no need for autoalloc.
But always check whether the allocation was successful!

@Andreas: can you give some example when to free memory?

Another thing is indeed the example Scott mentioned.

    
    
    BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);  
    if(!mat) return false;   
      
    //Is not the same thing as this  
      
    BaseMaterial *mat = nullptr;  
    if(condition) 
    
    
    { mat = BaseMaterial::Alloc(Mmaterial);
    
    
     if(!mat) return false; 
    
    
    }

What is the difference and what is the best way?

Again, I need to think in detail about all the information I got.
In the mean time, I have some good code snippets I can use now.

@Scott, thanks for joining in the  discussion and giving all the information and asking the good questions.
@Andreas, thanks for all the information and patience.

[Helper]

On 04/11/2016 at 10:15, xxxxxxxx wrote:

Hi Pim

Originally posted by xxxxxxxx

I thought the main advantage if AutoAlloc is, that you need not to worry about freeing memory.
I know now - reading all the posts above - that you only need to worry about freeing memory if you are the owner.
If cinema is the owner, for example after an insert, you need not to worry about freeing memory.
So, if you are going to insert, no need for autoalloc.

Right, I'd just not put the last sentence so black and white. Check the goodUseOfAutoAlloc() example in my last point. Although the material gets inserted later on, it still makes sense to use AutoAlloc. It's more about how many places you have (e.g. error exits), where you'd need to pay attention to free the material before inserting it into the document.

Originally posted by xxxxxxxx

But always check whether the allocation was successful!

Yes, please 

Originally posted by xxxxxxxx

@Andreas: can you give some example when to free memory?

Hm? Actually I think by now we have several such examples in this thread.
If you allocate memory, you are responsible to free it, except when transferring the ownership to somebody else (like C4D).

So, let's ignore AutoAlloc for a moment, maybe it adds too much to your confusion.

Bool AddMyGreatMaterial(BaseDocument* doc)
{
    BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);
    if(!mat)
        return false; 
  
    // ...adding shaders to the material...
    // in the process an error occurs
    if (error when adding shader)
    {
         // Here the material needs to be freed,
         // otherwise the material is lost (you are owner,
         // but you'll loose the pointer after returning from the function),
         // in consequence you'd have a memory leak 
        BaseMaterial::Free(mat);
        return false;
    }
    // ... imagine the function to me more complex. More allocations, more error exits...
  
    // ... and finally the success case:
    doc->InsertMaterial(mat);
    return true;

Of course there are a plethora of other cases, where you might need to free memory. For example on functions, where the docs say, that the caller owns the returned object. Unless you pass the ownership on to somebody else, you'd be responsible.
It all comes down to ownership.
If you are the owner and don't need the memory anymore, you have to free it.
If you are the owner and are about to loose the reference to that memory (pointer), you either need to free it or should think of another place to store the pointer.

Maybe these links on Memory Leaks help to shed some more light. Just my first finds, I'm sure by a little more searching you can find way better ones: stackoverflow: "What is a memory leak",  Memory Leak in Wikipedia and Microsoft's contribution to the topic.

Originally posted by xxxxxxxx

// Version A
BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);
if(!mat) return false; 
 
//Is not the same thing as this
 
// Version B
BaseMaterial *mat = nullptr;
if(condition) 
{ mat = BaseMaterial::Alloc(Mmaterial);
 if(!mat) return false; 
}
// HERE: mat needs to be checked for nullptr again!

What is the difference and what is the best way?

I'm not sure I can answer this question. After all this example is so stripped down, that I don't see a best way. It depends what you want to achieve.
The difference is in version A, you have a allocated Material pointed to by mat after those two lines of code.
In version B, it depends on the condition, if you have a valid mat pointer after those lines.

But I'm really not sure, that's what you were asking for.

In the end code is always also a matter of taste, just like spoken language. So the best way might depend on the eye of the coder... with one difference, regardless of what the best way looks like, it has to be correct. And that's also true for version B as long as you add another nullptr check after the scope of the if.

I know, memory allocations and memory leaks can be a tough topic. So I suggest, you take some time to digest this thread and maybe also take the chance to read some more on these topics. Afterwards the entire AutoAlloc topic will be a breeze and you'll easily see the benefits.