Why this code (assigning a material to a SUMaterialInput) triggers an access violation?

,

Hi all,
the following code using slapi triggers an access violation on the line result.material = material;. the color array passed as a parameter is { 0.5,0.5,0.5 };

I don’t see why but my c++ is a bit rusty so I may have missed something.
( The code is inspired from an answer from @bugra but I can’t find the link right now. )

SUMaterialInput createMaterial(std::vector<float> const color) {
    
    SUColor sucolor;
    sucolor.red   = static_cast<SUByte>(color[0] * 255);
    sucolor.green = static_cast<SUByte>(color[1] * 255);
    sucolor.blue  = static_cast<SUByte>(color[2] * 255);

    SUMaterialRef material;
    SUMaterialCreate(&material);
    SUMaterialSetColor(material,  &sucolor);  

    SUMaterialInput result;
    result.material = material;
    return result;
}

ok, got it. for others if you encouter this kind of bug.

SUMaterialRef material;

should be

SUMaterialRef material = SU_INVALID;

When you encounter things like this it’s advisable to check the return value from the SU* functions - making sure they return SU_ERROR_NONE.

When I inserted your code into a sample program I added some extra checks:

SUMaterialInput createMaterial(std::vector<float> const color)
{
  SUColor sucolor;
  sucolor.red   = static_cast<SUByte>(color[0] * 255);
  sucolor.green = static_cast<SUByte>(color[1] * 255);
  sucolor.blue  = static_cast<SUByte>(color[2] * 255);

  SUMaterialRef material;
  SUResult result = SUMaterialCreate(&material);
  assert(result == SU_ERROR_NONE);

  SUMaterialSetColor(material,  &sucolor);
  assert(result == SU_ERROR_NONE);

  SUMaterialInput material_input;
  material_input.material = material;
  return material_input;
}

When I then ran it I could see that SUMaterialCreate failed:

The reason is that you haven’t properly initialized your SUMaterialRef: SUMaterialRef material = SU_INVALID;

Complete working code:

// SLAPI-Test.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

#include <assert.h>
#include <iostream>
#include <vector>

#include "SketchUpAPI/sketchup.h"


SUMaterialInput createMaterial(std::vector<float> const color)
{
  SUColor sucolor;
  sucolor.red   = static_cast<SUByte>(color[0] * 255);
  sucolor.green = static_cast<SUByte>(color[1] * 255);
  sucolor.blue  = static_cast<SUByte>(color[2] * 255);

  SUMaterialRef material = SU_INVALID;
  SUResult result = SUMaterialCreate(&material);
  assert(result == SU_ERROR_NONE);

  SUMaterialSetColor(material,  &sucolor);
  assert(result == SU_ERROR_NONE);

  SUMaterialInput material_input;
  material_input.material = material;
  return material_input;
}


int _tmain(int argc, _TCHAR* argv[])
{
  SUInitialize();

  SUModelRef model = SU_INVALID;
  SUResult result = SUModelCreate(&model);
  assert(result == SU_ERROR_NONE);

  SUEntitiesRef entities = SU_INVALID;
  result = SUModelGetEntities(model, &entities);
  assert(result == SU_ERROR_NONE);

  std::vector<float> color = { 0.2f, 0.0f, 0.8f };
  auto material_input = createMaterial(color);

  SUModelRelease(&model);

  SUTerminate();
  return 0;
}

I often add asserts to make sure I can quickly trace these sort of things as otherwise problem propagate further down the pipe if one assumes the API functions always succeed.

The API samples uses this macro: #define SU_CALL(func) if ((func) != SU_ERROR_NONE) throw std::exception()

Then it wraps the various calls SU_CALL(SULoopInputCreate(&loop));

Note that has it’s own pitfalls, if you don’t use scoped pointers to handle your memory allocations then no cleanup will run when the exception is thrown. But you could employ asserts instead to aid your debugging. Though in general I’d advice on wrapper functions that properly check return values.

(Btw, it’d be preferable to post examples with a main function so one can just hit compile.)

1 Like