TommyKaneko / Sketchup-API-C-Wrapper

A complete set of C++ Wrapper classes for SketchUp C API objects
MIT License
29 stars 8 forks source link

Warnings: Returning address of local variable #41

Closed thomthom closed 6 years ago

thomthom commented 6 years ago

This is producing errors - and will not work. Either it should return a const ref of the internal member, or a Ref copy.

SUFaceRef Face::ref() const {  return SUFaceFromEntity(m_entity); }

Face::operator SUFaceRef() const {  return this->ref();}

Face::operator SUFaceRef*() {
  // TODO: test if the solution below works.
  SUFaceRef face = this->ref();
  return &face;
}

image

thomthom commented 6 years ago

I'm changing the Visual Studio project to treat warnings as errors.

TommyKaneko commented 6 years ago

A const pointer would be ideal. This method is useful for passing the wrapper object directly to the C API functions. Mostly for [in] parameters.

thomthom commented 6 years ago

Ref objects are so light-weight that I don't see any problem passing by value.

TommyKaneko commented 6 years ago

OK, lose the method altogether. Am happy to fix where it breaks.

thomthom commented 6 years ago

C doesn't have const refs, so I don't think you could use it directly anyway. The copy by value would be used instead.

jimfoltz commented 6 years ago

So here is a place where it brakes: Edge.cpp line 284.

Do you need to create a new vector<SUEdgeRef> to pass to the API function, or what is the syntax to go from vector<Edge> to SUEdgeRef *?

std::vector<Edge> Entities::add_edges(std::vector<Edge>& edges) {
  if (!SUIsValid(m_entities)) {
    throw std::logic_error("CW::Entities::add_edges(): Entities is null");
  }
  SUResult res = SUEntitiesAddEdges(m_entities, edges.size(), edges[0]);
  assert(res == SU_ERROR_NONE);

  // Transfer ownership of each edge
  for (auto& edge : edges)
      edge.attached(true);

  return edges;
}
thomthom commented 6 years ago

Yea, looks like it needs a temporary vector with the SUEdgeRef.

Even though Edge have implicit operator to SUEdgeRef, edges[0] gives the address of the first Edge in the vector. Do sure you can iterate that asSUEdgeRefs. I'm assuming you are seeing some access violations?

(Side note, prefer vector.data() over vector[0])