Preventing buffer overflow when using gets()

[lindsay.wilson.88]

Hi,

Quick Q about buffer overflows. I'm using the hardware UART and gets() to read a string in, then atoi() to convert to a number. Something like this:

[dyeatman]

Making the buffer longer is one way or you could use input.c in the drivers directory that has a lot of options including length checking.
_________________
Google and Forum Search are some of your best tools!!!!

[temtronic]

also
consider using a 'timed gets() '.

if not 'timed' and the serial connection fails(either sending PC stops, wire breaks or no 'EOT' ), PIC stays in a forever loop !
CCS FAQ shows one way to do this 'timed input'.

[Ttelmah]

Take a deep breath.

Everyone is forgetting the simple answer.
Load "input.c", and instead of using gets, use get_string.

This is a standard C function offered in this library. It is provided to deal
with exactly this problem. It behaves exactly like gets, _except_ is allows
you to specify a maximum number of characters.

On the 65535 limit, consider inputting the number into an int32 instead
of an int16, and then adding a test if the value is > 65535, and display
an error if so.

[dyeatman]

Yep. That's what I suggested.

[lindsay.wilson.88]

Many thanks for the tips. I've been having a play with the functions in input.c and they definitely are better! I'm having trouble understanding something though - if I send more characters over the serial port than I'm reading with the micro, why aren't they held in the buffer until next time? Meh, that doesn't make any sense. Try again:

Suppose I send 123456789 and a <CR> from computer to the PIC. If I do get_string(uart_text,5) and print the result, it receives "1234". Fair enough (although, why is it only four characters? Is the 5 including the null termination at the end?). Now, since get_string() is just using getc() repeatedly to read characters, I would have thought that once it had read the final "4", the PIC's hardware buffer would be able to store a couple more before running out of space, and I thought that these might be read by the next use of get_string(). But that doesn't seem to be the case.

Where did the characters go? Hope that makes sense.

Ehh....something's weird. If I do this:

[jeremiah]

Your printf takes time (lots of time) so by the time you are finished printing, the hardware buffer has filled up and overflowed, which means you just "miss" any characters that come after that. So you read 1 character, then print 3 (the character itself, the \r and the \n). Depending on your pic, the buffer is only so big. Most of the PIC24's I use have only a 4 byte buffer. Since you are only seeing up to 4 characters, my guess is your pic has a 4 byte hardware buffer.

[Ttelmah]

The get_string does not return until the <CR> is seen. It deliberately
disposes of characters beyond the specified space up to the CR.

As a general comment on storage, when you are doing something else,
you can expand the buffer available.
The hardware buffer is (normally) just under two characters.
1.9999 effectively.

When you setup the serial with #use_rs232, you can specify a software
expanded buffer.
RECEIVE_BUFFER=xx

If you set this to (say) 16, then you have a total of nearly 18 characters
of storage.

Your code must have an enable_interrupts(GLOBAL); line to make this
work. It sets up a INT_RDA handler to store the extra characters into
a buffer. Obviously costs the RAM for this and the code to add the handler
routines.

[lindsay.wilson.88]

@jeremiah - that makes total sense now, thanks!

@ttelmah - I just had a closer look at what get_string is doing and you're right, it reads and gets rid of everything after the max characters ;-) That's a much better way to do it, and I'll be sticking with get_string() from now on. Although I don't need it writing characters back so I made a copy of the function and deleted that part.

Regarding using a larger receive buffer and interrupts, is there any risk of that screwing up the timing of other interrupts? I was aiming to only have the Timer 2 interrupt running and ensuring that there's always enough free time for the rest of the code to read in serial data. I was a little bit confused by the list of interrupts that's shown in the device header file. There's obvious ones like INT_TIMER2, INT_CCP1 etc. But something like INT_RDA - since the actual flag is called RCIF, I was expecting something similar. At least they're all shown in the help file ;-)

[jeremiah]

Depending on the PIC you are using you can potentially set the timer interrupt to a higher priority than the serial interrupt, allowing it to interrupt the interrupt. Just don't share variables between them directly.

[Ttelmah]

OK.
As Jeriemiah says, you can use priorities, but these generally only change
the order the interrupts are serviced. On DsPIC's interrupts can interrupt
each other (if you enable nested interrupts), and on the PIC18, a high
priority interrupt can interrupt a low priority one (but only in a single
case). Normally the priority alters the order the interrupt flags are polled
when the main handler is called.

Avoiding using the serial interrupt is potentially better if timing of the
timer2 is critical.

RDA = Receive Data Available.
RCIF = Received Character Interrupt Flag.

There are a couple of really useful, but often 'missed' files it is well worth
looking at:

ints.txt
fuses.txt
readme.txt

These are all int the compiler directory, and contain useful refernences
to 'stuff'. The readme, used to be very well updated, but recently CCS
seem to only be noting very major changes in this.

[lindsay.wilson.88]

Mmmm....sounds like I'd better just stick with the one interrupt.

Re: those text files, AWESOME! I never would have figured out to look there - I have to say, much as I like the compiler compared with the others I've tried (like the great stinking behemoth that is MPLAB 🤣) some of the documentation is a just a wee bit spotty.

[Ttelmah]

The manual is very good, but several other things are needed to make it all
work.:

1) This forum!...
2) Those files.
3) The header for each processor.

The latter in particular is essential reading.

[jeremiah]

[Ttelmah]

Yes, I should have said on the 16bit PIC's rather than DsPIC's.