0

As already noted here using volatile std::string isn't a good idea.

I'm developing an application on FreeRTOS and I need to have a string which is moved between tasks. There is one task which receives commands through UART and can be asked by other tasks to get the response on a specified command. I want to use std::move on a string to make the application optimal.

Is there a neat and fast replacement of volatile std::string or do I have to implement a class with volatile field on my own? Maybe this approach is bad and I should use another structure to handle moving around responses on the commands?

EDIT: Here's some code.

I get the single bytes of commands through interrupt. The commands are human readable commands terminated with \r.

void rx_interrupt(char c)
{
    if(c == '\r')
    {
        c == '\0')
        BaseType_t higher_prior_task_woken = pdFALSE;
        vTaskNotifyGiveFromISR(rx_task_handle, &higher_prior_task_woken);
        portYIELD_FROM_ISR(higher_prior_task_woken);
    }
    rx_buf.push_byte(c);
}

rx_buf is a circular buffer which allows to pop whole commands as std::string.

Then the rx_task:

for (;;)
{
    auto notif_num = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
    while (notif_num--)
    {
        auto comm = rx_buf.pop_command();
        if (comm.length() == 0)
            continue;

        if (is_unsolicited_command(comm))
            handle_unsolicited_command(std::move(comm));
        if (is_awaited_command(comm))
            handle_awaited_command(std::move(comm));
    }
}

The rx_task is needed because I must firstly check whether an asynchronous event occurred which is indicated by an unsolicited command.

The received (awaited) commands may be long so I want to move them.

void handle_awaited_command(std::string &&cmd)
{
    os_lockguard guard(var_mux);
    if (!awaiting)
        return;

    awaited_command = std::move(cmd); // Problematic line
    xSemaphoreGive(cmd_received_sem);
    awaited_cmd_handled = true;
}

Finally any of the other task may await a command:

std::string get_command()
{
    os_lockguard guard_glob(mux);

    {
        os_lockguard guard(var_mux);
        awaiting = true;
    }

    xSemaphoreTake(cmd_received_sem, timeout);

    {
        os_lockguard guard(var_mux);
        if(awaited_cmd_handled)
            return std::move(awaited_command); // Problematic line
        else
            return std::string("");
    }
}

The thing is that the definitions looks like that:

volatile bool awaiting;
volatile bool awaited_cmd_handled;
volatile std::string awaited_command;

So I have here a volatile std::string.

K. Koovalsky
  • 596
  • 4
  • 17
  • *Maybe this approach is bad and I should use another structure to handle moving around responses on the commands?* I agree with this statement. Also I recommend you to look at `std::bind`. – 273K Aug 24 '18 at 07:09
  • Why not use a concurrent queue to hand over the string? – Alexander Oh Aug 24 '18 at 07:17
  • @S.M. could you be more specific? – K. Koovalsky Aug 24 '18 at 07:21
  • 2
    You should've posted some code example to illustrate what is going on instead of vogue text description. It is not clear why would you ever need a `volatile std::string`. There is no need for string to be volatile to be movable. – user7860670 Aug 24 '18 at 07:41
  • I suppose your tasks are functional objects. `std::bind` is useful for binding such tasks with some parameters (std::string in your case that are supposed to be immutable). Each `std::bind` clones a parameter, thus each task has own copy of an immutable string. When one task requests a string parameter from another task, a cloned string is returned. – 273K Aug 24 '18 at 07:43
  • 2
    @S.M. I personally find that a lambda is always superior to `std::bind`; they can do the same and more and I also find them much more readable – Jesper Juhl Aug 24 '18 at 08:32
  • @VTT ok, code has been added. – K. Koovalsky Aug 24 '18 at 19:12
  • Is this your code? Why are they defined as `volatile`? – user7860670 Aug 24 '18 at 19:31
  • @VTT why you ask me whether its my code? Yes, it is. The variables are defined as volatile to make sure that both tasks (`rx_task` and the one which awaits a command) work on up-to-date variables. – K. Koovalsky Aug 25 '18 at 10:53
  • The fix may be different depending on whether this is your code or third party code. If this code is yours then you can simply remove `volatile` from those declarations (note that `volatile` does not make concurrent access to variable safe in any way). If this code comes from third party then you can either switch to use something else or use a `const_cast` to cast away `volatile` and send an issue report to the third party. – user7860670 Aug 25 '18 at 11:09
  • Why I can be sure that the compiler won't optimize reads and writes to the global string? Could you explain why it should work? – K. Koovalsky Aug 25 '18 at 11:17

0 Answers0