So I've been working on this program for a while and I have gotten it to a point where it compiles just fine but when I run it I get a Segmentation Fault. I've backtraced the fault via gdb to the function below but cannot for the life of me see where the problem is, and I am not versed enough with gdb to determine any more details on the origin of the fault. Like the question says I'm trying to read from a file into a dynamically allocated linked list where each node of the list has a pointer to a struct called Element. Then I need to convert that linked list to an array of pointers to struct Element. I hope that a fresh pair of eyes can see some mistake I've made that I have been blind to.
UPDATE: Added rest of source files. Added gdb backtrace snippet.
Element.h
#ifndef ELEMENT_H
#define ELEMENT_H
#include <string>
using std::string;
struct Element
{
string name;
int atomNum;
double mass;
string abbr; // abbreviation
};
Element **get_table(string file, int& size);
#endif /* ELEMENT_H */
get_table.cpp
#include "Element.h"
#include <iostream>
#include <fstream>
#include <string>
using namespace std;
struct Node
{
Element *elem;
Node *next;
};
Element **get_table(string filename, int &size)
{
ifstream infile;
string nextline;
size = 0;
infile.open(filename.c_str());
Node *head = NULL;
if(infile)
{
do{
getline(infile, nextline, ' ');
Node *node = new Node;
node->elem = new Element;
node->elem->atomNum = stoi(nextline.c_str());
getline(infile, nextline, ' ');
node->elem->abbr = nextline;
getline(infile, nextline, ' ');
node->elem->mass = stod(nextline.c_str());
getline(infile, nextline);
node->elem->name = nextline;
node->next = head;
head = node;
size++;
}while(infile);
}
else
{
infile.close();
return NULL;
}
//convert linked list into array of pointers
Element **table = new Element*[size];
int i = 0;
Node *curr = head;
while(curr->next != NULL)
{
table[i] = curr->elem;
curr = curr->next;
i++;
}
//deallocate linked list
while(head != NULL)
{
Node* temp = head->next;
delete head;
head = temp;
}
infile.close();
return table;
}
periodic_table.cpp
#include <cstdlib>
#include <iostream>
#include <iomanip>
#include "Element.h"
using namespace std;
int compare_by_name(const void* e1, const void* e2)
{
struct Element *left = *(Element**) e1;
struct Element *right = *(Element**) e2;
if (left->name < right->name)
return -1;
else if (left->name > right->name)
return 1;
else
return 0;
}
int printTable(Element** tab, const int size)
{
cout << "Name" << setw(23) << "Abr" << setw(4) << "Ano" << setw(7) << "Mass" << endl;
cout << endl << "----------------------- --- --- ------" << endl;
cout << fixed;
for (int i = 0; i < size; i++) {
cout << setw(23) << left << tab[i]->name;
cout << setw(3) << left << tab[i]->abbr;
cout << setprecision(0) << setw(4) << right << tab[i]->atomNum;
cout << setprecision(2) << setw(7) << right << tab[i]->mass << endl;
}
return 0;
}
double getAvgMass(Element** table, const int size)
{
double sum = 0.0;
if (size == 0)
return sum;
else
for (int i = 0; i < size; i++) {
sum += table[i]->mass;
}
return sum / size;
}
/*
*
*/
int main()
{
string filename = "/user/tvnguyen7/data/periodictable.dat";
string tempfile = "periodictable.dat";
filename = tempfile;
int size = 0;
Element **table = get_table(filename, size);
if (table != NULL) //sort table by element name and print in alphabetical order
{
qsort(table, size, sizeof (Element*), compare_by_name);
printTable(table, size);
//calculate and display average mass of elements in table
cout << setw(23) << left << "The average mass =" << fixed << setprecision(2) << setw(14) << right << getAvgMass(table, size) << endl;
}
delete [] table;
table = nullptr;
return 0;
}
FIXED: gdb backtrace
(gdb) run
Starting program: /user/jscanning/proj2.out
terminate called after throwing an instance of 'std::invalid_argument'
what(): stoi
Program received signal SIGABRT, Aborted.
0x00007ffff712ff65 in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x00007ffff712ff65 in raise () from /lib64/libc.so.6
#1 0x00007ffff7131c8a in abort () from /lib64/libc.so.6
#2 0x00007ffff7a93ecd in __gnu_cxx::__verbose_terminate_handler() ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libstdc++.so.6
#3 0x00007ffff7a91d36 in ?? ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libstdc++.so.6
#4 0x00007ffff7a91d81 in std::terminate() ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libstdc++.so.6
#5 0x00007ffff7a91f98 in __cxa_throw ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libstdc++.so.6
#6 0x00007ffff7abb2cf in std::__throw_invalid_argument(char const*) ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libstdc++.so.6
#7 0x000000000040232f in __gnu_cxx::__stoa<long, int, char, int> (
__convf=0x401490 <strtol@plt>, __name=0x402573 "stoi", __str=0x7fffffffdd50 "\n", __idx=0x0)
at /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/include/g++-v5/ext/string_conversions.h:65
#8 0x0000000000402258 in std::__cxx11::stoi (__str="\n", __idx=0x0, __base=10)
at /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/include/g++-v5/bits/basic_string.h:5260
#9 0x0000000000401fa2 in get_table (filename="/user/tvnguyen7/data/periodictable.dat",
size=@0x7fffffffdfa4: 118) at get_table.cpp:54
#10 0x0000000000401ab0 in main () at periodic_table.cpp:78
(gdb)
periodictable.dat
1 H 1.00794 Hydrogen
2 He 4.002602 Helium
3 Li 6.941 Lithium
4 Be 9.012182 Beryllium
5 B 10.811 Boron
6 C 12.0107 Carbon
7 N 14.0067 Nitrogen
8 O 15.9994 Oxygen
9 F 18.9984032 Fluorine
10 Ne 20.1797 Neon
11 Na 22.98976928 Sodium
12 Mg 24.305 Magnesium
13 Al 26.9815386 Aluminum
14 Si 28.0855 Silicon
15 P 30.973762 Phosphorus
16 S 32.065 Sulfur
17 Cl 35.453 Chlorine
18 Ar 39.948 Argon
19 K 39.0983 Potassium
20 Ca 40.078 Calcium
21 Sc 44.955912 Scandium
22 Ti 47.867 Titanium
23 V 50.9415 Vanadium
24 Cr 51.9961 Chromium
25 Mn 54.938045 Manganese
26 Fe 55.845 Iron
27 Co 58.933195 Cobalt
28 Ni 58.6934 Nickel
29 Cu 63.546 Copper
30 Zn 65.38 Zinc
31 Ga 69.723 Gallium
32 Ge 72.64 Germanium
33 As 74.9216 Arsenic
34 Se 78.96 Selenium
35 Br 79.904 Bromine
36 Kr 83.798 Krypton
37 Rb 85.4678 Rubidium
38 Sr 87.62 Strontium
39 Y 88.90585 Yttrium
40 Zr 91.224 Zirconium
41 Nb 92.90638 Niobium
42 Mo 95.96 Molybdenum
43 Tc 98 Technetium
44 Ru 101.07 Ruthenium
45 Rh 102.9055 Rhodium
46 Pd 106.42 Palladium
47 Ag 107.8682 Silver
48 Cd 112.411 Cadmium
49 In 114.818 Indium
50 Sn 118.71 Tin
51 Sb 121.76 Antimony
52 Te 127.6 Tellurium
53 I 126.90447 Iodine
54 Xe 131.293 Xenon
55 Cs 132.9054519 Cesium
56 Ba 137.327 Barium
57 La 138.90547 Lanthanum
58 Ce 140.116 Cerium
59 Pr 140.90765 Praseodymium
60 Nd 144.242 Neodymium
61 Pm 145 Promethium
62 Sm 150.36 Samarium
63 Eu 151.964 Europium
64 Gd 157.25 Gadolinium
65 Tb 158.92535 Terbium
66 Dy 162.5 Dysprosium
67 Ho 164.93032 Holmium
68 Er 167.259 Erbium
69 Tm 168.93421 Thulium
70 Yb 173.054 Ytterbium
71 Lu 174.9668 Lutetium
72 Hf 178.49 Hafnium
73 Ta 180.94788 Tantalum
74 W 183.84 Tungsten
75 Re 186.207 Rhenium
76 Os 190.23 Osmium
77 Ir 192.217 Iridium
78 Pt 195.084 Platinum
79 Au 196.966569 Gold
80 Hg 200.59 Mercury
81 Tl 204.3833 Thallium
82 Pb 207.2 Lead
83 Bi 208.9804 Bismuth
84 Po 209 Polonium
85 At 210 Astatine
86 Rn 222 Radon
87 Fr 223 Francium
88 Ra 226 Radium
89 Ac 227 Actinium
90 Th 232.03806 Thorium
91 Pa 231.03588 Protactinium
92 U 238.02891 Uranium
93 Np 237 Neptunium
94 Pu 244 Plutonium
95 Am 243 Americium
96 Cm 247 Curium
97 Bk 247 Berkelium
98 Cf 251 Californium
99 Es 252 Einsteinium
100 Fm 257 Fermium
101 Md 258 Mendelevium
102 No 259 Nobelium
103 Lr 262 Lawrencium
104 Rf 267 Rutherfordium
105 Db 268 Dubnium
106 Sg 271 Seaborgium
107 Bh 272 Bohrium
108 Hs 270 Hassium
109 Mt 276 Meitnerium
110 Ds 281 Darmstadtium
111 Rg 280 Roentgenium
112 Cn 285 Copernicium
113 Uut 284 Ununtrium
114 Uuq 289 Ununquadium
115 Uup 288 Ununpentium
116 Uuh 293 Ununhexium
117 Uus 294 Ununseptium
118 Uuo 294 Ununoctium
UPDATE: When I compile with -g and -o I get this:
/tmp/cccoH75A.o: In function `main':
/user/jscanning/periodic_table.cpp:81: undefined reference to `get_table(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int&)'
collect2: error: ld returned 1 exit status
What does this mean?
The good news is you were really, really close. The bad news is the "really really close" part -- it only takes one subtle error to torpedo your code.
First the general discussion. While there is nothing wrong with using a pointer-to-pointer-to-Element, you are missing the benefits of using C++ and the automatic memory handling offered by <vector>. That said, it is good to know how to handle both.
Your error is:
while(curr->next != NULL)
Here, backing though your linked-list to assign pointers to table, you fail to iterate and include Hydrogen, stopping one element prior at Helium. This leaves your last pointer as an Uninitialized (Indeterminate) Pointer. When you pass the elements to qsort, you SegFault. To insure proper inclusion of H, you need:
while(curr != NULL)
(curr contains valid data until it is NULL, not curr->next = NULL)
Further, as discussed your read is open to failing due to not checking each call to getline. A better approach would be to read each line from the file into nextline and then use stringstream to separate the values into temporary variables before allocating each node. There you validate your read and validate your parse before allocating memory. You could do something like:
Element **get_table (string filename, int& size)
{
ifstream infile;
string nextline;
size = 0;
infile.open(filename.c_str());
Node *head = NULL;
if (!infile.is_open()) {
cerr << "file open failed '" << filename << "'\n";
return NULL;
}
while (getline (infile, nextline)) {
stringstream s (nextline);
string name, abbr;
int atomNum;
double mass;
if ((s >> atomNum >> abbr >> mass >> name)) {
Node *node = new Node;
node->elem = new Element;
node->elem->atomNum = atomNum;
node->elem->abbr = abbr;
node->elem->mass = mass;
node->elem->name = name;
node->next = head;
head = node;
size++;
}
else {
/* handle error or warn on failed parse as desired */
}
}
Next just a note about hardcoding values -- don't. If you need constants (including filenames), then #define them at the top of your source file, so there is one convenient location to change the values if your needs change, e.g.
periodic_table.cpp
#include "Element.h"
#define FNAME "dat/periodictable.dat"
...
int main(void) {
int size = 0;
Element **table = get_table (FNAME, size);
Putting it altogether, your code provides a nicely formatted periodic table, e.g.
Example Use/Output
$ ./bin/periodic_table
Name Abr Ano Mass
----------------------- --- --- ------
Actinium Ac 89 227.00
Aluminum Al 13 26.98
Americium Am 95 243.00
Antimony Sb 51 121.76
Argon Ar 18 39.95
Arsenic As 33 74.92
Astatine At 85 210.00
...
Ununtrium Uut 113 284.00
Uranium U 92 238.03
Vanadium V 23 50.94
Xenon Xe 54 131.29
Ytterbium Yb 70 173.05
Yttrium Y 39 88.91
Zinc Zn 30 65.38
Zirconium Zr 40 91.22
The average mass = 146.44
(you can tweak the output field width (setw(24)) to tidy up your column spacing under the heading)
Look things over and let me know if you have questions.
..and if you want to know the secret, high-powered debugging technique that disclosed the error?
while (curr->next != NULL)
{
cerr << "assigning table[" << i << "]: " << curr->elem->abbr << '\n';
table[i] = curr->elem;
curr = curr->next;
i++;
}
I simply put output the abbreviations assigned to table and it stopped at:
assigning table[116]: He
Segmentation fault (core dumped)
Code As-Used in Example Above
Element.h
Don't define using xxx in headers. That will include the entire namespace in each file that includes the header.
#ifndef ELEMENT_H
#define ELEMENT_H
#include <string>
struct Element
{
std::string name;
int atomNum;
double mass;
std::string abbr; // abbreviation
};
Element **get_table (std::string file, int& size);
#endif /* ELEMENT_H */
get_table.cpp
#include "Element.h"
#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
#include <vector>
using namespace std;
struct Node
{
Element *elem;
Node *next;
};
Element **get_table (string filename, int& size)
{
ifstream infile;
string nextline;
size = 0;
infile.open(filename.c_str());
Node *head = NULL;
if (!infile.is_open()) {
cerr << "file open failed '" << filename << "'\n";
return NULL;
}
while (getline (infile, nextline)) {
stringstream s (nextline);
string name, abbr;
int atomNum;
double mass;
if ((s >> atomNum >> abbr >> mass >> name)) {
Node *node = new Node;
node->elem = new Element;
node->elem->atomNum = atomNum;
node->elem->abbr = abbr;
node->elem->mass = mass;
node->elem->name = name;
node->next = head;
head = node;
size++;
}
}
//convert linked list into array of pointers
Element **table = new Element*[size];
int i = 0;
Node *curr = head;
while (curr != NULL) /* error here - curr, not curr->next */
{
// cerr << "table[" << i << "]: " << curr->elem->abbr << '\n';
table[i] = curr->elem;
curr = curr->next;
i++;
}
//deallocate linked list
while(head != NULL)
{
Node* temp = head->next;
delete head;
head = temp;
}
infile.close();
return table;
}
periodic_table.cpp
#include <cstdlib>
#include <iostream>
#include <iomanip>
#include "Element.h"
#define FNAME "dat/periodictable.dat"
using namespace std;
int compare_by_name(const void* e1, const void* e2)
{
struct Element *left = *(Element**) e1;
struct Element *right = *(Element**) e2;
if (left->name < right->name)
return -1;
else if (left->name > right->name)
return 1;
else
return 0;
}
int printTable(Element** tab, const int size)
{
cout << "Name" << setw(23) << "Abr" << setw(4) << "Ano" <<
setw(7) << "Mass" << endl;
cout << endl << "----------------------- --- --- ------" << endl;
cout << fixed;
for (int i = 0; i < size; i++) {
cout << setw(24) << left << tab[i]->name;
cout << setw(3) << left << tab[i]->abbr;
cout << setprecision(0) << setw(4) << right << tab[i]->atomNum;
cout << setprecision(2) << setw(7) << right << tab[i]->mass << endl;
}
return 0;
}
double getAvgMass(Element** table, const int size)
{
double sum = 0.0;
if (size == 0)
return sum;
else
for (int i = 0; i < size; i++) {
sum += table[i]->mass;
}
return sum / size;
}
/*
*
*/
int main() {
int size = 0;
Element **table = get_table (FNAME, size);
if (table != NULL) //sort table alphabetically by element name and print
{
qsort (table, size, sizeof (Element*), compare_by_name);
printTable(table, size);
//calculate and display average mass of elements in table
cout << setw(24) << left << "The average mass =" <<
fixed << setprecision(2) << setw(14) << right <<
getAvgMass(table, size) << endl;
}
delete [] table;
table = nullptr;
return 0;
}