Update File.cpp for use with fifo (nonblock read)

[MichelDeVittori]

Solved reading problem with dabmux configuration 'nonblock true' and 'inputfile fifo' e.g.:

subchannels { sub-data { id 6 type packet nonblock true inputfile "/tmp/dabdata.fifo" protection 1 bitrate 32 } }

[mpbraendli]

Hello Michel,

thanks for the pull request! Indeed it doesn't look right, because the case where there is a short read is not handled correctly, i.e. when ret is neither 0 nor required_len. In that situation, both the original code and your fix show wrong behaviour.

I believe (but I'm not sure) that the behaviour should be to fill into m_nonblock_buffer until it's at least size bytes large, and only then copy size bytes into buffer, and preserve the remaining bytes in m_nonblock_buffer.

Do you agree with this assessment and would you be ok to fix that issue?

Again many thanks for reaching out mpb

[MichelDeVittori]

Hi, yes you are right, on Monday I will fix it and upload the full solution.

Regards, MDV

Da: Matthias P. Braendli notifications@github.com Inviato: Freitag, 19. Juli 2019 14:13 A: Opendigitalradio/ODR-DabMux ODR-DabMux@noreply.github.com Cc: Michel De Vittori michel.devittori@gmail.com; Author author@noreply.github.com Oggetto: Re: [Opendigitalradio/ODR-DabMux] Update File.cpp (#38)

Hello Michel,

thanks for the pull request! Indeed it doesn't look right, because the case where there is a short read is not handled correctly, i.e. when ret is neither 0 nor required_len. In that situation, both the original code and your fix show wrong behaviour.

I believe (but I'm not sure) that the behaviour should be to fill into m_nonblock_buffer until it's at least size bytes large, and only then copy size bytes into buffer, and preserve the remaining bytes in m_nonblock_buffer.

Do you agree with this assessment and would you be ok to fix that issue?

Again many thanks for reaching out mpb

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Opendigitalradio/ODR-DabMux/pull/38?email_source=notifications&email_token=AF5CGPXZHF5OBHK4YXTXASLQAGVVTA5CNFSM4IFEBH3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2LOQCA#issuecomment-513206280 , or mute the thread https://github.com/notifications/unsubscribe-auth/AF5CGPSOAWTAEGLUZLSD7P3QAGVVTANCNFSM4IFEBH3A . https://github.com/notifications/beacon/AF5CGPVJZF7P5N52RALNAE3QAGVVTA5CNFSM4IFEBH3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2LOQCA.gif

[MichelDeVittori]

I leave the code to force to read the required ammount of data, this means that the reader is not really non-blocking.

if (size > m_nonblock_buffer.size()) { size_t m_nonblock_buffer_len = m_nonblock_buffer.size(); size_t required_len = size - m_nonblock_buffer_len; size_t current_len = 0; std::vector buf(required_len);

while(current_len != required_len) {
    ret = read(m_fd, buf.data()+current_len, required_len-current_len);

    /* If no process has the pipe open for writing, read() shall return 0
    * to indicate end-of-file. */
    if (ret == 0) {
        return 0;
    }

    /* If some process has the pipe open for writing and O_NONBLOCK is
    * set, read() shall return −1 and set errno to [EAGAIN]. */
    if (ret == -1 and errno == EAGAIN) {
        return 0;
    }
    else if (ret == -1) {
        etiLog.level(alert) << "ERROR: Can't read file " << strerror(errno);
        return -1;
    }
    else {
        current_len  += ret;
    }
}

std::copy(m_nonblock_buffer.begin(), m_nonblock_buffer.end(), buffer);
buffer += m_nonblock_buffer_len;
m_nonblock_buffer.clear();
std::copy(buf.begin(), buf.end(), buffer);
return required_len+m_nonblock_buffer_len;

}

[mpbraendli]

Hi, thanks for your patch. I've simplified it a bit, and the case when read() returns less bytes than requested is not also handled. Can you please check if the latest next branch works for you?