Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake-build-debug/
cmake-build-release/
build/
build**/
.idea/
93 changes: 50 additions & 43 deletions src/DiskFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,64 +100,57 @@
**/
#include "DiskFile.h"
#include "StdAfx.h"
#include <fstream>
#include <iostream>

CDiskFile::CDiskFile(CConfigurator *cfg, CSystem *sys, CDiskController *c,
int idebus, int idedev)
: CDisk(cfg, sys, c, idebus, idedev) {

/* If the filename exists in the config, use that,
* otherwise, use a default filename */
filename = myCfg->get_text_value("file");
if (!filename) {
FAILURE_1(Configuration, "%s: Disk has no file attached!\n", devid_string);
defaultFilename = std::string(devid_string) + ".default.img";
std::cerr << devid_string
<< ": Disk has no filename attached! Assuming default: "
<< defaultFilename << std::endl;
filename = const_cast<char *>(defaultFilename.c_str());
}

#ifdef HAVE_FOPEN64
handle = fopen64(filename, read_only ? "rb" : "rb+");
#else
handle = fopen(filename, read_only ? "rb" : "rb+");
#endif

if (!handle) {
printf("%s: Could not open file %s!\n", devid_string, filename);

int sz = myCfg->get_num_value("autocreate_size", false, 0) / 1024 / 1024;
if (!sz)
FAILURE_1(Runtime, "%s: File does not exist and no autocreate_size set",
/* if the file does not exist, create it. ifs actually checks for
* accessibility, not for existence, but for compatibility with
* platforms without c++17/boost and std::filesystem::exists, this
* is good enough. ifstream.good() returns false if the file does
* not exist.*/
std::ifstream ifs(filename, std::ios::binary);
if (!ifs.good()) {
std::cerr << devid_string << ": file does not exist: " << filename
<< std::endl;

/* If the disk file size was not set and the disk file does not exist, do
* not create it, but exit */
int diskFileSize = myCfg->get_num_value("autocreate_size", false, 0);
if (!diskFileSize) {
FAILURE_1(Runtime, "%s: file does not exist and no autocreate_size set.",
devid_string);

void *crt_buf;
handle = fopen_large(filename, "wb");
if (!handle)
FAILURE_1(Runtime, "%s: File does not exist and could not be created",
devid_string);
crt_buf = calloc(1024, 1024);
printf("%s: writing %d 1kB blocks: 0%%\b\b\b\b", devid_string, sz);

int lastpc = 0;
for (int a = 0; a < sz; a++) {
fwrite(crt_buf, 1024, 1024, handle);

int pc = a * 100 / sz;
if (pc != lastpc) {
printf("%3d\b\b\b", pc);
lastpc = pc;
}

fflush(stdout);
}

printf("100%%\n");
fclose(handle);
free(crt_buf);
if (read_only)
handle = fopen_large(filename, "rb");
else
handle = fopen_large(filename, "rb+");
if (!handle) {
FAILURE_1(Runtime, "%s: File created could not be opened", devid_string);
/* Don't create disk file if it's supposed to be a cdrom */
if (is_cdrom) {
FAILURE_1(Runtime, "%s: file does not exist and is configured as cdrom.",
devid_string);
}

printf("%s: %d MB file %s created.\n", devid_string, sz, filename);
createDiskFile(filename, diskFileSize);
}

#ifdef HAVE_FOPEN64
handle = fopen64(filename, read_only ? "rb" : "rb+");
#else
handle = fopen(filename, read_only ? "rb" : "rb+");
#endif

// determine size...
fseek_large(handle, 0, SEEK_END);
byte_size = ftell_large(handle);
Expand Down Expand Up @@ -192,6 +185,20 @@ CDiskFile::CDiskFile(CConfigurator *cfg, CSystem *sys, CDiskController *c,
cylinders, heads, sectors);
}

void CDiskFile::createDiskFile(const std::string &fileName, int diskFileSize) {

std::ofstream ofs(fileName, std::ios::binary);
if (ofs.is_open() && ofs.good()) {
ofs.seekp((diskFileSize)-1);
ofs.write("", 1);
} else {
FAILURE_1(Runtime, "%s: File does not exist and could not be created", devid_string);
}

std::cout << devid_string << " " << (diskFileSize / 1024 / 1024) << "MB file "
<< filename << " created" << std::endl;
}

CDiskFile::~CDiskFile(void) {
printf("%s: Closing file.\n", devid_string);
fclose(handle);
Expand Down
3 changes: 3 additions & 0 deletions src/DiskFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,8 @@ class CDiskFile : public CDisk {
protected:
FILE *handle;
char *filename;

void createDiskFile(const std::string &filename, int diskFileSize);
std::string defaultFilename;
};
#endif //! defined(__DISKFILE_H__)
9 changes: 7 additions & 2 deletions src/System.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,15 @@ void CSystem::ResetMem(unsigned int membits) {
/**
* Register a device.
**/
int CSystem::RegisterComponent(CSystemComponent *component) {
void CSystem::RegisterComponent(CSystemComponent *component) {
acComponents[iNumComponents] = component;
iNumComponents++;
return 0;
}

void CSystem::UnregisterComponent(CSystemComponent *component) {
if (acComponents[iNumComponents] == component)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if unregistering a component this way is a good idea - the destructors may be called in a different order that the constructors.

Maybe the component storing its number and checking for that would help with this?

(I don't generally like the component managing its registration in CSystem, but that's another issue.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think comparing by memory address is less likely to cause race condition when components are destructed i another order. A counter would have to be atomic, and a for loop to compare all items would have the same crash behavior.

Catching the exception at creation and then handling it would be another solution but that would also require some refactoring of the component registration (it happens so early in the base class, it be better if it was registering after constructing for this bug).

I could try to just not set it to a nullptr because it will be deleted right after the call to stop threads? Or try how a std::atomic counter would perform? What has your preference?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right - I missed that the component number couldn't be read from an uninitialized component. I take my comment back, you can keep the code as it is.

Copy link
Owner

@lenticularis39 lenticularis39 Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could try to just not set it to a nullptr because it will be deleted right after the call to stop threads? Or try how a std::atomic counter would perform? What has your preference?

I might be missing something, but doesn't the if before the nullptr setting check the wrong index? iNumComponents is increased in CSystem::RegisterComponent, so it should point to the next (empty) element, not to the last one. If that's the case then the setting to nullptr isn't needed, because it clearly works without it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right, it checks the wrong component. The entire check can be removed, I'll do that right away.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's done, now it just decrements the counter.

acComponents[iNumComponents] = nullptr;
iNumComponents--;
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/System.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ struct SConfig {
* The ES40 emulator has the following chipset configuration:
* - 1 x 21274-C1 Cchip (controller chip) - The Cchip controls the other chips
*in the chipset, as well as the DRAM memory array in a system. The Cchip
*interfaces with the CPUs command and address buses.
*interfaces with the CPU's command and address buses.
* - 8 x 21274-D1 Dchip (data slice chip) - The Dchips interface with the
*system data bus and provide the data path between the CPU, DRAM memory, and
*the Pchip(s).
Expand Down Expand Up @@ -241,7 +241,8 @@ class CSystem {

int RegisterMemory(CSystemComponent *component, int index, u64 base,
u64 length);
int RegisterComponent(CSystemComponent *component);
void RegisterComponent(CSystemComponent *component);
void UnregisterComponent(CSystemComponent *component);
int RegisterCPU(class CAlphaCPU *cpu);

CSystem(CConfigurator *cfg);
Expand Down
1 change: 1 addition & 0 deletions src/SystemComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ CSystemComponent::CSystemComponent(CConfigurator *cfg, CSystem *system) {
* destructor.
**/
CSystemComponent::~CSystemComponent() {
cSystem->UnregisterComponent(this);
free(devid_string);
devid_string = nullptr;
}