0

I need to send a message from Main thread to my Created Thread using WinAPI and Ring Buffer.

I defined structures and wrote functions for my Ring buffer. Ring Buffer - it contains head, tail, size and pointer to the structure Descriptor which has length of Data and data itself. As I need to send 2 parameters to CreateThread function, I created the third structure ThreadParams to keep 2 parameters.

I want to leave this structures how they are now, not changeable.

typedef struct _Descriptor
{
    uint32_t dataLen;
    void * data;
} Descriptor;

typedef struct _ringBuffer
{
    Descriptor *bufferData;
    int head;
    int tail;
    int size;
} ringBuffer;

typedef struct _ThreadParams
{
    void * ptr1;
    void * ptr2;
} ThreadParams;

There are my realisations of Ring Buffer functions:

void bufferFree(ringBuffer *buffer)
{
    free(buffer->bufferData);
}

void ringInitialization(ringBuffer *buffer, int size)
{
    buffer->size = size;
    buffer->head = 0;
    buffer->tail = 0;
    buffer->bufferData = (Descriptor*)malloc(sizeof(Descriptor) * size);
}

int pushBack(ringBuffer *buffer, void * data) // fill buffer
{
    buffer->bufferData[buffer->tail++] = *(Descriptor*)data;
    if (buffer->tail == buffer->size)
    {
        buffer->tail = 0;
    }
    return 0;
}

int popFront(ringBuffer *buffer)
{
    if (buffer->head != buffer->tail)
    {
        buffer->head++;
        if (buffer->head == buffer->size)
        {
            buffer->head = 0;
        }
    }
    return 0;
}

My main: I checked that I can send a few bytes (the memory is shared between threads), now I need to send a big message (> BUFF_SIZE) though Ring Buffer what I'm trying to do in while() cycle. Here is the question: how should I do it? My thing doesn't work because I catch an exception in printf() function (memory acces violation).

#include <stdio.h>
#include <stdlib.h>
#include <conio.h>
#include <windows.h>
#include <strsafe.h>
#include <stdint.h>

#define RING_SIZE 256
#define BUFFER_SIZE 1024

DWORD WINAPI HandleSendThread(LPVOID params);

uint8_t * getPointer(uint8_t *buffer, uint32_t index)
{
    uint8_t * ptr = ((uint8_t*)buffer) + index * BUFFER_SIZE;
    return ptr;
}

int main(int argc, char * argv[])      
{
    //Descriptor * ringData = (Descriptor *)malloc(sizeof(Descriptor) * RING_SIZE);

    ringBuffer ring;
    ringInitialization(&ring, RING_SIZE);
    void * packetBuffer = malloc(BUFFER_SIZE * RING_SIZE);
    uint8_t * currentBuffer = getPointer(packetBuffer, 0);
    uint8_t * str = "Mr. and Mrs. Dursley, of number four, Privet Drive, were proud to say that they were perfectly normal, thank you very much. They were the last people you'd expect to be involved in anything strange or mysterious, because they just didn't hold with such nonsense. Mr.Dursley was the director of a firm called Grunnings, which made drills.He was a big, beefy man with hardly any neck, although he did have a very large mustache.Mrs.Dursley was thin and blonde and had nearly twice the usual amount of neck, which came in very useful as she spent so much of her time craning over garden fences, spying on the neighbors.The Dursleys had a small son called Dudley and in their opinion there was no finer boy anywhere.";

    strcpy(currentBuffer, str);
    ring.bufferData[0].data = currentBuffer;
    ring.bufferData[0].dataLen = strlen(str);

    int currentSize = 0;
    int ringSize = RING_SIZE;
    while(ring.bufferData[0].data != '\0')
    {
        for (int i = currentSize; i < ringSize; i + RING_SIZE)
        {
            pushBack(&ring, currentBuffer); 
            printf("h = %s, tail = %s, dataBuffer = %s\n", (char*)ring.head, (char*)ring.tail, (char*)ring.bufferData[i].data);
        }
        currentSize = ringSize;
        ringSize = 2 * ringSize;
        popFront(&ring);
    }

    ThreadParams params = { &ring, packetBuffer };
    HANDLE MessageThread = 0;
    MessageThread = CreateThread(NULL, 0, HandleSendThread, &params, 0, NULL);
    if (MessageThread == NULL)
    {
        ExitProcess(MessageThread);
    }

    WaitForSingleObject(MessageThread, INFINITE);
    CloseHandle(MessageThread);
    system("pause");
    return 0;
}

And my CreateThread function:

DWORD WINAPI HandleSendThread(LPVOID params)
{
    ringBuffer * ring = ((ThreadParams*)params)->ptr1;
    void * buffer = ((ThreadParams*)params)->ptr2;
    //ring->bufferData[0].dataLen = sizeof(buffer) + sizeof(ring->bufferData[0])*1024;

    printf("Shared memory check: ringBuffer data = \"%s\", \nlength = %d\n", (char*)ring->bufferData[0].data, ring->bufferData[0].dataLen);
    return 0;
}
wildplasser
  • 43,142
  • 8
  • 66
  • 109
  • 2
    `params` is a local variable, it no longer exists when the thread starts running. The ++ operator is read-modify-write and is therefore not thread-safe. – Hans Passant Jun 05 '17 at 10:12
  • 1
    'ring.bufferData[0].dataLen = strlen(str);' no, it's not. – ThingyWotsit Jun 05 '17 at 10:14
  • 1
    @HansPassant yes it is, but the main thread is stuck on the WaitForSingleObject and so the local var lifetime is OK. – ThingyWotsit Jun 05 '17 at 10:15
  • The other stuff, about non-thread-safe operations, however, is mucho valid:( – ThingyWotsit Jun 05 '17 at 10:16
  • 2
    I didn't scroll down far enough to see the wait. So there isn't any point in using a thread at all. Well, that should help solve the problem :) – Hans Passant Jun 05 '17 at 10:19
  • 1
    @HansPassant I haven't got my head, (or tail:), round how it works yet. There does not seem to be any 'data available' signalling, no semaphore or whatever, so I don't see what a consumer would do if there was no data;( – ThingyWotsit Jun 05 '17 at 10:25
  • @ThingyWotsit What I wanted to do: for example, my message has ~800 bytes. I want to read first BUF_SIZE bytesm, transmit, receive and print them, then the second part of data till it's == \0 (btw, is it correct to write \0?) – ezviagintcev Jun 05 '17 at 10:41
  • 1
    Unrelated to your issue, but [CreateThread](https://msdn.microsoft.com/en-us/library/windows/desktop/ms682453.aspx) insists that *"[a] thread in an executable that calls the C run-time library (CRT) should use the _beginthreadex and _endthreadex functions for thread management rather than CreateThread and ExitThread; [...]. If a thread created using CreateThread calls the CRT, the CRT may terminate the process in low-memory conditions."* – IInspectable Jun 05 '17 at 11:32
  • I tried to organize that with cycles but I came up to nothing, Guys, do you have any ideas how to use my ring buffer correctly? – ezviagintcev Jun 05 '17 at 13:28

2 Answers2

0

Your most immediate problem is the inconsistency between the code in pushBack(), which expects data to point to a Descriptor, and the code in your main function, which passes in a pointer to a string instead.

If you had declared pushBack() properly, i.e.,

void pushBack(ringBuffer *buffer, Descriptor * data)
{
    buffer->bufferData[buffer->tail++] = *data;
    if (buffer->tail == buffer->size)
    {
        buffer->tail = 0;
    }
}

Then the compiler would have been able to warn you about the discrepancy.

You also have an infinite loop here:

for (int i = currentSize; i < ringSize; i + RING_SIZE)

You probably meant

for (int i = currentSize; i < ringSize; i += RING_SIZE)

... although it still doesn't look to me like it will do anything sensible. Nor do I understand the purpose of the outer loop, which compares a pointer to a character.

Harry Johnston
  • 35,639
  • 6
  • 68
  • 158
0

Found a solution

int main(int argc, char * argv[])
{
    ringBuffer ring;
    ringInitialization(&ring, RING_SIZE);
    void * packetBuffer = malloc(BUFFER_SIZE * RING_SIZE);
    Descriptor temp = { 0 };
    uint8_t * currentBuffer = getPointer(packetBuffer, 0);
    uint8_t * str = "Mr. and Mrs. Dursley, of number four, Privet Drive, were proud to say that they were perfectly normal, thank you very much. They were the last people you'd expect to be involved in anything strange or mysterious, because they just didn't hold with such nonsense. Mr.Dursley was the director of a firm called Grunnings, which made drills.He was a big, beefy man with hardly any neck, although he did have a very large mustache.Mrs.Dursley was thin and blonde and had nearly twice the usual amount of neck, which came in very useful as she spent so much of her time craning over garden fences, spying on the neighbors.The Dursleys had a small son called Dudley and in their opinion there was no finer boy anywhere.";

    strcpy(currentBuffer, str);
    temp.dataLen = strlen(str);
    temp.data = currentBuffer;

    pushBack(&ring, &temp);
    ThreadParams params = { &ring, packetBuffer };
    HANDLE MessageThread = 0;
    MessageThread = CreateThread(NULL, 0, HandleSendThread, &params, 0, NULL);
    if (MessageThread == NULL)
    {
        ExitProcess(MessageThread);
    }
    WaitForSingleObject(MessageThread, INFINITE);
    CloseHandle(MessageThread);
    system("pause");
    return 0;
}

DWORD WINAPI HandleSendThread(LPVOID params)
{
    ringBuffer * ring = ((ThreadParams*)params)->ptr1;
    void * buffer = ((ThreadParams*)params)->ptr2;
    Descriptor * temp = &ring->bufferData[ring->head];

    for (int i = 0; i < temp->dataLen; i++)
    {
        printf("%c", ((char*)temp->data)[i]);
    }
    printf("\n");
    return 0;
}