This function cause the window to be blank then crashes

Question

I'm trying to find why thus code doesn't work, and after some testing I found that the function submitSprite has the cause of the crash in it, but I don't know how to fix it

#include "renderer.h"

Renderer newRenderer(SDL_Window* window)
{
    Renderer renderer;
    renderer.renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);

    return renderer;
}

void destroyRenderer(Renderer renderer)
{
    SDL_DestroyRenderer(renderer.renderer);
}

void submitSprite(Renderer renderer, Sprite sprite)
{
    SDL_Rect* rect;
    rect->x=sprite.x;
    rect->y=sprite.y;
    rect->w=sprite.w;
    rect->h=sprite.h;

    SDL_RenderCopy(renderer.renderer, sprite.texture, NULL, rect);
}

void render(Renderer renderer)
{
    SDL_RenderPresent(renderer.renderer);
    SDL_RenderClear(renderer.renderer);
}

renderer.renderer is an SDL_Renderer*

@Gabryx86_64 because it's just a pointer, and you never made it point at any allocated object... so you can't do anything with it and have undefined behaviour if you do. — underscore_d, Jan 29 '21 at 12:23
You should also consider whether you really want to pass such structures by value, as it will copy them, which if they are large is unnecessary and might slow performance. To avoid copying them, pass them by pointer-to-`const` (if you need to modify any, drop the `const`). — underscore_d, Jan 29 '21 at 12:24
You can be lucky that it crashed. This kind of error can have all kind of strange sideeffects. :) — Devolus, Jan 29 '21 at 12:27

Devolus · Accepted Answer · 2021-01-29T12:55:06.183

2

void submitSprite(Renderer renderer, Sprite sprite)
{
    SDL_Rect* rect;
    rect->x=sprite.x;
    rect->y=sprite.y;
    rect->w=sprite.w;
    rect->h=sprite.h;

    SDL_RenderCopy(renderer.renderer, sprite.texture, NULL, rect);
}

Here you access an pointer which was never initialized. So either use a local variable, or a allocate (and free) memory for it.

i.e.:

void submitSprite(Renderer renderer, Sprite sprite)
{
    SDL_Rect rect;
    rect.x=sprite.x;
    rect.y=sprite.y;
    rect.w=sprite.w;
    rect.h=sprite.h;

    SDL_RenderCopy(renderer.renderer, sprite.texture, NULL, &rect);
}

edited Jan 29 '21 at 12:55

answered Jan 29 '21 at 12:19

Devolus

21,661
13
66
113

1

As underscore_d also pointed out, you should use pointers instead of pass by value. Makes it much faster. ;) – Devolus Jan 29 '21 at 12:38

score 0 · Answer 2 · answered Jan 29 '21 at 18:12

I can't write a comment, but i'd just like to add, you should make one instance of an SDL_Renderer rather than keep creating one when you need it. As Devolus said, pass by pointer. Passing by value after creating one would also give you a memory leak if you forget to call SDL_DestroyRenderer(), which also adds to the slowness of that method. When you need a renderer, have the parameter SDL_Renderer* renderer

This function cause the window to be blank then crashes

2 Answers2