array/sprintf strange values?

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
lock attach
Attachments are accessible only for community members.
Anonymous
Not applicable

Hi All,

I am still fairly new to programming,  so I'm sorry in advance if my terminology/formatting isn't up to scratch.

I am currently working on an element of a musical instrument that will allow me to actuate a button multiple times with a varying time delay between presses, record this and loop the pattern.

I currently have an array for recording the timings of the button presses, I am using USB-UART/terra term to check my values are being recorded correctly but I am finding the array being populated with seemingly nonsensical values. I am wondering where this data is coming from.

Highlighted in red in the image below are the values that I expect to be retained in the T0, T1..etc values. In short - Mtapi is the int used in selecting what location in the array I want to store the current milliTAP value (T0, T1 etc..). But as you can see from the following data lines they are being replaced by 10 digit numbers.

Basically, I am wondering what those numbers are, why I am seeing them and how to stop them populating my array!

I did think they maybe something to do with data locations of the array instances? but honestly this is all new to me and I have no idea. That and how they are changing at what seems to be random to me.

Any help or direction to further reading is much appreciated.

Capture- terra.JPG

a higher-res copy of the above image here.

main.c code on GitHub here

for quick reference (shortened for posting here):-

if ((Mtapi==0)&&(MTap[0]>0)&&(MTapBtnCHK==0)){
     int i; for (int i=0; i<63; i++) {
        MTap[i]=0; 
          }
     } 

if
(MTapBtn_Read()==0){
     MultiTapTempo();
     }

if
(MTapBtn_Read()==1){
     MTapBtnCHK=0;
     }

//gap in code posted here, then:

void MultiTapTempo(void){
     if (MTapBtnCHK==0){
          Mtapi++;
          MTapBtnCHK=1; 
          if(Mtapi==0){}
          if(Mtapi==1){MTap[0]=millisMTAP;}
          if(Mtapi==2){MTap[1]=millisMTAP;}
          if(Mtapi==3){MTap[2]=millisMTAP;}
          if(Mtapi==4){MTap[3]=millisMTAP;}
          if(Mtapi==5){MTap[4]=millisMTAP;}
          if(Mtapi==6){MTap[5]=millisMTAP;}
          if(Mtapi==7){MTap[6]=millisMTAP;}
          if(Mtapi==8){MTap[7]=millisMTAP;}
          if(Mtapi==9){MTap[8]=millisMTAP;}
     }
0 Likes
1 Solution
KyTr_1955226
Level 6
Level 6
250 sign-ins 10 likes given 50 solutions authored

The first thing I notice is that your character array buff is only 64 bytes.  Your sprintf is trying to force a string much longer than 64 characters into that buffer:

sprintf(buff, "millis:%d BPM:%d  mux:%d millisTAP:%d tapbtnCHK:%d T1-%d T2-%d T3-%d TA-%d  Mtapi-%d Mtapbtnread-%d \r\n  T0-%d T1-%d T2-%d T3-%d T4-%d T5-%d T6-%d T7-%d T8-%d T9-%d T10-%d \r\n \r\n ", Decoder_1_Position,BPM, mux, millisTAP, TapBtnCHK, Tap1, Tap2, Tap3 , TapAvg , Mtapi,i, MTap[0], MTap[1],MTap[2],MTap[3],MTap[4],MTap[5],MTap[6],MTap[7],MTap[8],MTap[9],MTap[10]);

that's at least 184 characters worth of string right there (before even accounting for the values it will put in the formatters places) and buff only has 64 bytes allocated for itself.  Your values could very well be in the memory after buff.

sprintf cares not for your array bounds, and is just going to keep writing over anything in its path until it is done.  There's a decent chance your values could be getting clobbered.

See: c - character array overflowing by sprintf - Stack Overflow

Generally, overflowing your array bounds with sprintf is "Undefined Behavior"

View solution in original post

3 Replies
KyTr_1955226
Level 6
Level 6
250 sign-ins 10 likes given 50 solutions authored

The first thing I notice is that your character array buff is only 64 bytes.  Your sprintf is trying to force a string much longer than 64 characters into that buffer:

sprintf(buff, "millis:%d BPM:%d  mux:%d millisTAP:%d tapbtnCHK:%d T1-%d T2-%d T3-%d TA-%d  Mtapi-%d Mtapbtnread-%d \r\n  T0-%d T1-%d T2-%d T3-%d T4-%d T5-%d T6-%d T7-%d T8-%d T9-%d T10-%d \r\n \r\n ", Decoder_1_Position,BPM, mux, millisTAP, TapBtnCHK, Tap1, Tap2, Tap3 , TapAvg , Mtapi,i, MTap[0], MTap[1],MTap[2],MTap[3],MTap[4],MTap[5],MTap[6],MTap[7],MTap[8],MTap[9],MTap[10]);

that's at least 184 characters worth of string right there (before even accounting for the values it will put in the formatters places) and buff only has 64 bytes allocated for itself.  Your values could very well be in the memory after buff.

sprintf cares not for your array bounds, and is just going to keep writing over anything in its path until it is done.  There's a decent chance your values could be getting clobbered.

See: c - character array overflowing by sprintf - Stack Overflow

Generally, overflowing your array bounds with sprintf is "Undefined Behavior"

Anonymous
Not applicable

aha!
Thank you, this is great. I just changed my buffer to 640 character - simply a bigger number for testing, and it seems to be working as I had originally planned.

Do you think this is an acceptable solution, increasing the buff char length? or is there a more efficient technique I should look into?
I have a lot of reading on my hands I think as I need to better understand what exactly is going on there, rather than living in copy-paste land.

0 Likes

If increasing the buffer length works, it works.  But there are a couple things you could consider:

You could use snprintf which has an argument for buffer length.  If the length of the result is too large, it will return the difference between lengths.  You could pass in sizeof(buff) for the length.

You could also split up your string into more digestible chunks.  Rather than making a huge string of 200+ characters, you could split it into 4 or more individual strings of 64 and send them each one after the other.