473,396 Members | 1,738 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,396 software developers and data experts.

Bus error--not sure why

The following code compiles with gcc but returns a bus error when I run
the executable. Any ideas?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
/* generate serial number list*/
int main (void) {

char appname[] = "MYAPP"; //new app
char myserial[100]; //pointer variable for serial numbers
int count = 20000; //number of serial numbers
int i; //index number;
char *serials[count+1]; //full list of serial numbers
int seriallength; //length of serial number
char *serialblock; //block of memory for serial number

//trim application name
char *shortname = (char *) malloc(2);
strncpy(shortname, appname, 2);
free(shortname);

//build the serial number array
for (i = 1; i < count; ++i) {
int five = i*5; //first number of the serial array
int eleven = i/11; //second number
int one = (i-1); //third number
sprintf(myserial, "%s-%i-%i-%i-%s", appname, five, eleven, one,
shortname);//assign component values to serial number string
seriallength=strlen(myserial); //length of serial number
serialblock = (char *) malloc(seriallength + 1); //allocate memory
for serial
if (serialblock == NULL) {
return 0; //check for null memory
}

strcpy(serialblock, myserial); //copy data to address of serial
block from serial
serials[i]=serialblock; //assign data at serial block address to
next indeas of serials array

free(serialblock); //free memory
}

for (i = 1; i < count; ++i) {
printf(" %s\n", i, serials[i]);
}

return 0;

}

--
Kevin Walzer
Code by Kevin
http://www.codebykevin.com
Jul 25 '08 #1
11 1476
Kevin Walzer <kw@codebykevin.comwrote:
The following code compiles with gcc but returns a bus error when I run
the executable. Any ideas?
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
/* generate serial number list*/
int main (void) {
char appname[] = "MYAPP"; //new app
char myserial[100]; //pointer variable for serial numbers
int count = 20000; //number of serial numbers
int i; //index number;
char *serials[count+1]; //full list of serial numbers
Why 'count+1' when your lop only runs from 1 to 'count-1'?
int seriallength; //length of serial number
char *serialblock; //block of memory for serial number
//trim application name
char *shortname = (char *) malloc(2);
strncpy(shortname, appname, 2);
free(shortname);
The last three lines are a NOP. You allocate memory, copy
something to it and deallocate it (and casting the return
value of malloc() doesn't make much sense).
//build the serial number array
for (i = 1; i < count; ++i) {
int five = i*5; //first number of the serial array
int eleven = i/11; //second number
int one = (i-1); //third number
sprintf(myserial, "%s-%i-%i-%i-%s", appname, five, eleven, one,
shortname);//assign component values to serial number string
You can't use 'shortname' anymore, you already deallocated what
it was pointing to. And even if you hadn't it would not be a
string since there is no trailing '\0' character (please read
the description of strncpy() again and carefully note what hap-
pens if the source string is longer than what you allow to be
copied). So even then using "%s" isn't an option since it re-
quires a string. If you wouldn't have deallocated the memory
you would have been able to print it out using "%.2s" (to keep
printf() from using more than the first two chars). But then
you could get the same effect by using "%.2s" with 'appname',
so the whole use of 'shortname' seems to be rather useless.
seriallength=strlen(myserial); //length of serial number
serialblock = (char *) malloc(seriallength + 1); //allocate memory
for serial
if (serialblock == NULL) {
return 0; //check for null memory
}
strcpy(serialblock, myserial); //copy data to address of serial
block from serial
serials[i]=serialblock; //assign data at serial block address to
next indeas of serials array
free(serialblock); //free memory
And now 'serials[i]' points to free-ed memory you aren't allowed
to use for anything at all. You seem to assume that just holding
a pointer to something would keep what it points to in existence,
but that's not the case. (Perhaps you're too much used to a lan-
guage with a GC?) One way to go would be to not deallocate
'serialblock' but just keep the pointer stored in the 'serials'
array and deallocate the memory only when you don't need it
anymore (e.g. after the printf() later on).
}
for (i = 1; i < count; ++i) {
printf(" %s\n", i, serials[i]);
And here you use pointers to memory that already has been deallocated
again and again.
}
return 0;
Are you sure you're getting a bus error (I guess that means a SIGBUS)
instead of a segmentation fault (SIGSEGV)?

Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de
Jul 25 '08 #2
On Thu, 24 Jul 2008 20:35:41 -0400, Kevin Walzer <kw@codebykevin.com>
wrote:
>The following code compiles with gcc but returns a bus error when I run
the executable. Any ideas?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
/* generate serial number list*/
int main (void) {

char appname[] = "MYAPP"; //new app
char myserial[100]; //pointer variable for serial numbers
int count = 20000; //number of serial numbers
int i; //index number;
char *serials[count+1]; //full list of serial numbers
int seriallength; //length of serial number
char *serialblock; //block of memory for serial number

//trim application name
char *shortname = (char *) malloc(2);
Don't cast the return from malloc. It doesn't help anything and can
suppress a diagnostic message you would really want to see.
strncpy(shortname, appname, 2);
This invokes undefined behavior by writing beyond you allocated
memory. appname contains 6 char. You allocated space for 2. The
adage about 10 pounds in a 5 pound bag applies.
free(shortname);
On many installations, writing beyond allocated memory messes up
internal allocation control tables and free gets very confused.

What is the point of these three statements? You allocate memory,
copy into it, and then free it without accomplishing anything.
>
//build the serial number array
for (i = 1; i < count; ++i) {
int five = i*5; //first number of the serial array
int eleven = i/11; //second number
int one = (i-1); //third number
sprintf(myserial, "%s-%i-%i-%i-%s", appname, five, eleven, one,
shortname);//assign component values to serial number string
More undefined behavior. You freed shortname. You cannot attempt to
access the memory it used to point to. In fact, you cannot evaluate
the value it contains for any purpose.
seriallength=strlen(myserial); //length of serial number
serialblock = (char *) malloc(seriallength + 1); //allocate memory
for serial
if (serialblock == NULL) {
return 0; //check for null memory
It is a little strange to return success when your program failed.
}

strcpy(serialblock, myserial); //copy data to address of serial
block from serial
This is why you should not use // comments in a usenet post.
serials[i]=serialblock; //assign data at serial block address to
next indeas of serials array

free(serialblock); //free memory
This releases the memory that serial[i] points to.
}

for (i = 1; i < count; ++i) {
printf(" %s\n", i, serials[i]);
And this invokes undefined behavior because you no longer own the
memory serial[i] used to point to.
> }

return 0;

}

Remove del for email
Jul 25 '08 #3
Barry Schwarz wrote:
On Thu, 24 Jul 2008 20:35:41 -0400, Kevin Walzer <kw@codebykevin.com>
wrote:
>The following code compiles with gcc but returns a bus error when I run
the executable. Any ideas?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
/* generate serial number list*/
int main (void) {

char appname[] = "MYAPP"; //new app
char myserial[100]; //pointer variable for serial numbers
int count = 20000; //number of serial numbers
int i; //index number;
char *serials[count+1]; //full list of serial numbers
int seriallength; //length of serial number
char *serialblock; //block of memory for serial number

//trim application name
char *shortname = (char *) malloc(2);

Don't cast the return from malloc. It doesn't help anything and can
suppress a diagnostic message you would really want to see.
> strncpy(shortname, appname, 2);

This invokes undefined behavior by writing beyond you allocated
memory. appname contains 6 char. You allocated space for 2. The
adage about 10 pounds in a 5 pound bag applies.
... but putting not more than two char of a six-char
string is fine (unless malloc() returned NULL). Note the
use of strncpy() instead of strcpy() -- of course, the two
characters { 'M', 'Y' } do not constitute a '\0'-terminated
string, but that scarcely matters here because the next
thing this pointless piece of code does is
> free(shortname);
[remainder of helpful analysis snipped]
--
Eric Sosman
es*****@ieee-dot-org.invalid
Jul 25 '08 #4
Jens Thoms Toerring wrote:
Kevin Walzer <kw@codebykevin.comwrote:
The last three lines are a NOP. You allocate memory, copy
something to it and deallocate it (and casting the return
value of malloc() doesn't make much sense).
<snip code>
You can't use 'shortname' anymore, you already deallocated what
it was pointing to. And even if you hadn't it would not be a
string since there is no trailing '\0' character (please read
the description of strncpy() again and carefully note what hap-
pens if the source string is longer than what you allow to be
copied). So even then using "%s" isn't an option since it re-
quires a string. If you wouldn't have deallocated the memory
you would have been able to print it out using "%.2s" (to keep
printf() from using more than the first two chars). But then
you could get the same effect by using "%.2s" with 'appname',
so the whole use of 'shortname' seems to be rather useless.
<snip code>
>
And now 'serials[i]' points to free-ed memory you aren't allowed
to use for anything at all. You seem to assume that just holding
a pointer to something would keep what it points to in existence,
but that's not the case. (Perhaps you're too much used to a lan-
guage with a GC?) One way to go would be to not deallocate
'serialblock' but just keep the pointer stored in the 'serials'
array and deallocate the memory only when you don't need it
anymore (e.g. after the printf() later on).
<snip code>
>
And here you use pointers to memory that already has been deallocated
again and again.
Thank you for your detailed response to my code. It was very helpful and
enabled me to solve the problems I was encountering.

Yes, I am still learning the rules for memory management. I have a lot
of experience with dynamic/scripting languages (Tcl and Python) but very
little with C.

For the record, here is the code I came up with, which compiles and runs
without error:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
/* regproc.c--generate serial number list*/

int main (void) {

/* new app */
char appname[] = "MYAPP";

/* pointer variable for serial numbers */
char myserial[100];

/* number of serial numbers */
int count = 20000;

/* index number */
int i;

/* full array of serial numbers */
char *serials[count];

/* length of individual serial number */
int seriallength;

/* placeholder for individual serial number in serials array */
char *serialnumber;

/* character buffer for abbreviated app name: first two letters plus \0
character */
char shortname[3];

/* now trim the application name to the first two letters, copy to
"shortname" array, append to the serial number later */
strncpy(shortname, appname, 2);

/* now build the serial number array */
for (i = 1; i < count; ++i) {
/* second item of the array; first is the app name */
int five = i*5;
/* third item of the array */
int eleven = i/11;
/* fourth item of the array */
int one = (i-1);
/* dump all serial number components to "myserial" character
array--appname, five, eleven, one, shortname */
sprintf(myserial, "%s-%i-%i-%i-%s", appname, five, eleven, one,
shortname);
/* get length of "myserial" string */
seriallength=strlen(myserial);

/* allocate memory for individual serial numbers in array, check for
null */
serialnumber = (char *) malloc(seriallength);
if (serialnumber == NULL) {
return 0; //check for null memory
}

/* copy value of "myserial" to the address of "serialnumber" element */

strcpy(serialnumber, myserial);

/* assign data at serial block address to next indeas of serials array */
serials[i]=serialnumber;

} /* end of array loop */
/*loop through array, print out values of serial numbers */
for (i = 1; i < count; ++i) {
printf("%s\n", serials[i]);
}

/*displose of memory block address for "serialnumber" */

free(serialnumber);
return 0;

}
--
Kevin Walzer
Code by Kevin
http://www.codebykevin.com
Jul 25 '08 #5
Kevin Walzer <kw@codebykevin.comwrote:
Which unfortunately not always means that it's correct...
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

/* regproc.c--generate serial number list*/

int main (void) {

/* new app */
char appname[] = "MYAPP";

/* pointer variable for serial numbers */
char myserial[100];

/* number of serial numbers */
int count = 20000;

/* index number */
int i;

/* full array of serial numbers */
char *serials[count];

/* length of individual serial number */
int seriallength;

/* placeholder for individual serial number in serials array */
char *serialnumber;

/* character buffer for abbreviated app name: first two letters plus \0
character */
char shortname[3];

/* now trim the application name to the first two letters, copy to
"shortname" array, append to the serial number later */
strncpy(shortname, appname, 2);
That still doesn't add a '\0' to the end of 'shortname' even though
there's now enough space for it. You probably were lucky that
'shortname' was initialized to all zero but you shouldn't bet
on that always to happen. So you have to add

sfortname[ 2 ] = '\0';
/* now build the serial number array */
for (i = 1; i < count; ++i) {
/* second item of the array; first is the app name */
int five = i*5;
/* third item of the array */
int eleven = i/11;
/* fourth item of the array */
int one = (i-1);
/* dump all serial number components to "myserial" character
array--appname, five, eleven, one, shortname */
sprintf(myserial, "%s-%i-%i-%i-%s", appname, five, eleven, one,
shortname);
While all this is fine I probably would shorten that to (but
that's just a matter of taste!):

seriallength = sprintf( myserial, "%s-%i-%i-%i-%.2s", appname,
5 * i, i / 11, i - 1, appname );

sprintf() returns the number of chars it wrote (excluding the
trailing '\0') if it succeeded, so you also don't need the call
of strlen() later.
/* get length of "myserial" string */
seriallength=strlen(myserial);

/* allocate memory for individual serial numbers in array, check for null
*/
serialnumber = (char *) malloc(seriallength);
Here's still a problem. You need one more byte of memory then
what strlen() did return since the trailing (and required)
'\0' at the end of the string isn't included in the count.

Another thing: casting the return value of malloc() isn't
needed in C. It can even be counterproductive since it will
keep the compiler from warning you when you forgot to include
<stdlib.h>. And that in turn can lead to a hard to find bug.
if (serialnumber == NULL) {
return 0; //check for null memory
}

/* copy value of "myserial" to the address of "serialnumber" element */

strcpy(serialnumber, myserial);

/* assign data at serial block address to next indeas of serials array */
serials[i]=serialnumber;
While this is fine in principle it's a bit more work that necessary.
You can also do

if ( ( strings[ i ] = malloc( seriallength + 1 ) ) == NULL )
return 0;
strcpy( serials[ i ], myserial );

So the extra 'serialnumber' variable isn't needed and you can
avoid an extra assignment.
} /* end of array loop */
/*loop through array, print out values of serial numbers */
for (i = 1; i < count; ++i) {
printf("%s\n", serials[i]);
}

/*displose of memory block address for "serialnumber" */

free(serialnumber);
This is also not correct. It only would deallocate the memory
you allocated last, but all the other strings would still be
allocated. Why not simply do

for ( i = 1; i < count; ++i ) {
printf( "%s\n", serials[ i ] );
free( serials[ i ] );
}

That way you get rid of the memory for each string the moment
you don't need it anymore.

Putting everything together I probably would shorten your program
to (the less lines of code you have the less errors you can make;-):

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define COUNT 20000

int main ( void ) {
char appname[ ] = "MYAPP";
char myserial[ 100 ];
int i;
char *serials[ COUNT ];
int len;

for ( i = 1; i <= COUNT; i++ ) {
len = sprintf( myserial, "%s-%d-%d-%d-%.2s", appname,
5 * i, i / 11, i - 1, appname );
if ( ( strings[ i - 1 ] = malloc( len + 1 ) ) == NULL )
return 0;
strcpy( serials[ i - 1 ], myserial );
}

for ( i = 0; i < COUNT; ++i ) {
printf( "%s\n", serials[ i ] );
free( serials[ i ] );
}

return 0;
}

I also changed it so that it creates COUNT serial numbers instead
of COUNT - 1 since that looks more logical to me.

Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de
Jul 25 '08 #6
jt@toerring.de (Jens Thoms Toerring) writes:

(This is reallt to the OP, but it builds on all the correct things
you've said so...)
Kevin Walzer <kw@codebykevin.comwrote:
<snip program>
Putting everything together I probably would shorten your program
to (the less lines of code you have the less errors you can make;-):

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define COUNT 20000

int main ( void ) {
char appname[ ] = "MYAPP";
char myserial[ 100 ];
int i;
char *serials[ COUNT ];
int len;

for ( i = 1; i <= COUNT; i++ ) {
len = sprintf( myserial, "%s-%d-%d-%d-%.2s", appname,
5 * i, i / 11, i - 1, appname );
if ( ( strings[ i - 1 ] = malloc( len + 1 ) ) == NULL )
return 0;
strcpy( serials[ i - 1 ], myserial );
}

for ( i = 0; i < COUNT; ++i ) {
printf( "%s\n", serials[ i ] );
free( serials[ i ] );
}

return 0;
}
I can't see why the OP needs all the effort of storing these things
just to print them. All that is needed is a loop with your sprintf as
printf. No big array, no mallocs or frees.

Of course, if this is part of some bigger program that needs them all,
then there may be some point, but even then they are not hard to make
on demand.

--
Ben.
Jul 25 '08 #7
Thank you for tweaking the code yet again. I will review it carefully.

--
Kevin Walzer
Code by Kevin
http://www.codebykevin.com
Jul 25 '08 #8
Ben Bacarisse wrote:
>
I can't see why the OP needs all the effort of storing these things
just to print them. All that is needed is a loop with your sprintf as
printf. No big array, no mallocs or frees.

Of course, if this is part of some bigger program that needs them all,
then there may be some point, but even then they are not hard to make
on demand.
The obvious reason for my effort is that I am still feeling my way
through C, and have not grasped the best way to do certain things. If I
were an expert, I would not be posting here with questions.

--
Kevin Walzer
Code by Kevin
http://www.codebykevin.com
Jul 25 '08 #9
Kevin Walzer <kw@codebykevin.comwrites:
Ben Bacarisse wrote:
>I can't see why the OP needs all the effort of storing these things
just to print them. All that is needed is a loop with your sprintf as
printf. No big array, no mallocs or frees.

Of course, if this is part of some bigger program that needs them all,
then there may be some point, but even then they are not hard to make
on demand.

The obvious reason for my effort is that I am still feeling my way
through C, and have not grasped the best way to do certain things. If
I were an expert, I would not be posting here with questions.
I hope you did not take offence. When I said I could not see why you
were doing it I was not being snide or sarcastic, just reporting that
I though I was missing some point. Maybe I should have said: "Unless
this is part of larger program, it would be simpler just to print
these rather than store them".

--
Ben.
Jul 25 '08 #10
Ben Bacarisse wrote:
>
I hope you did not take offence. When I said I could not see why you
were doing it I was not being snide or sarcastic, just reporting that
I though I was missing some point. Maybe I should have said: "Unless
this is part of larger program, it would be simpler just to print
these rather than store them".
No, no offense taken. Thanks.

--
Kevin Walzer
Code by Kevin
http://www.codebykevin.com
Jul 26 '08 #11
On Fri, 25 Jul 2008 00:17:52 -0400, Eric Sosman
<es*****@ieee-dot-org.invalidwrote:
>Barry Schwarz wrote:
>On Thu, 24 Jul 2008 20:35:41 -0400, Kevin Walzer <kw@codebykevin.com>
wrote:
>>The following code compiles with gcc but returns a bus error when I run
the executable. Any ideas?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
/* generate serial number list*/
int main (void) {

char appname[] = "MYAPP"; //new app
char myserial[100]; //pointer variable for serial numbers
int count = 20000; //number of serial numbers
int i; //index number;
char *serials[count+1]; //full list of serial numbers
int seriallength; //length of serial number
char *serialblock; //block of memory for serial number

//trim application name
char *shortname = (char *) malloc(2);

Don't cast the return from malloc. It doesn't help anything and can
suppress a diagnostic message you would really want to see.
>> strncpy(shortname, appname, 2);

This invokes undefined behavior by writing beyond you allocated
memory. appname contains 6 char. You allocated space for 2. The
adage about 10 pounds in a 5 pound bag applies.

... but putting not more than two char of a six-char
string is fine (unless malloc() returned NULL). Note the
use of strncpy() instead of strcpy() -- of course, the two
Yes, even the presence of the third parameter was insufficient for me
to notice the n.
>characters { 'M', 'Y' } do not constitute a '\0'-terminated
string, but that scarcely matters here because the next
thing this pointless piece of code does is
>> free(shortname);
[remainder of helpful analysis snipped]

Remove del for email
Jul 26 '08 #12

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

4
by: Oliver Spiesshofer | last post by:
Hi, I want to make a standard-function that handles "Are you sure"-type questions. In my idea it should look something like this: <code> Function deleteitem($item) {
1
by: BKM | last post by:
I've been using the following 2 ways to make sure my WebBrowser is finished loading a page before continuing with the next code. do:doevents:loop while WebBrowser.Busy do:doevents:loop until...
1
by: Jeff Uchtman | last post by:
Query in ASP against a MS SQL DB. <% Dim Sql Dim Conn Dim totalCount Dim rsCount Dim rsACount Dim PetACount Dim rsBCount
1
by: Sean Berry | last post by:
I saw some sample code of whay I think was wither js or html... can't remember. But, it would create the effect of a yellow popup window that appeared when you left you mouse button over a link. ...
7
by: Nathan Sokalski | last post by:
I have a form that allows the user to upload a file. Even though <input type="file" runat="server"> is intended to have the user choose the file using the browse button, it still allows them to...
37
by: yogpjosh | last post by:
Hello All, I was asked a question in an interview.. Its related to dynamically allocated and deallocated memory. eg. //start char * p = new char; ...
9
by: Chad | last post by:
The input file is: Params 1 2 3 4 5 6,7 Data 1,1
6
by: Alex Maghen | last post by:
Let's say I have a web forms button which performs a Delete or something. I'd like a click on the button to pop an "Are you Sure" dialog on the client side and then only execute the button's...
2
by: Henry | last post by:
I am trying to work my way through a textbook on web services. I was running a help page and got this error message. To me it is vague. Not knowing the code well enough I am not sure which data...
3
by: sheldonlg | last post by:
I have a page that has three submit buttons. Depending upon which one was clicked, I process it differently on the server in php. One of those buttons is a delete action so I would like to add an...
0
by: Charles Arthur | last post by:
How do i turn on java script on a villaon, callus and itel keypad mobile phone
0
by: ryjfgjl | last post by:
In our work, we often receive Excel tables with data in the same format. If we want to analyze these data, it can be difficult to analyze them because the data is spread across multiple Excel files...
0
by: emmanuelkatto | last post by:
Hi All, I am Emmanuel katto from Uganda. I want to ask what challenges you've faced while migrating a website to cloud. Please let me know. Thanks! Emmanuel
0
BarryA
by: BarryA | last post by:
What are the essential steps and strategies outlined in the Data Structures and Algorithms (DSA) roadmap for aspiring data scientists? How can individuals effectively utilize this roadmap to progress...
1
by: Sonnysonu | last post by:
This is the data of csv file 1 2 3 1 2 3 1 2 3 1 2 3 2 3 2 3 3 the lengths should be different i have to store the data by column-wise with in the specific length. suppose the i have to...
0
by: Hystou | last post by:
There are some requirements for setting up RAID: 1. The motherboard and BIOS support RAID configuration. 2. The motherboard has 2 or more available SATA protocol SSD/HDD slots (including MSATA, M.2...
0
marktang
by: marktang | last post by:
ONU (Optical Network Unit) is one of the key components for providing high-speed Internet services. Its primary function is to act as an endpoint device located at the user's premises. However,...
0
by: Hystou | last post by:
Most computers default to English, but sometimes we require a different language, especially when relocating. Forgot to request a specific language before your computer shipped? No problem! You can...
0
agi2029
by: agi2029 | last post by:
Let's talk about the concept of autonomous AI software engineers and no-code agents. These AIs are designed to manage the entire lifecycle of a software development project—planning, coding, testing,...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.