Thread: Quick Problem Check

Originally Posted by hamster_nz

One day, after many hard nights of debugging where a few bytes have gone missing in the middle of a stream, or your program has terminated with a write error when you have plenty of disk space left, or your user suspends your program with CTRL+Z you will look back on this with fond memories...

For write, it is common to just keep on trying the remaining buffer, until either all bytes are written, or a different error is received. Something like:

Code:

   size_t written = 0;
   while(length > 0) {
      size_t bytes = write(df, buffer, length);

      if(bytes == -1) {
         if(errno == EINTR)
            continue;
         return -1;  // Or however you want to handle error
      } else {
         buffer  += bytes;
         length  -= bytes;  
         written += bytes;
      }
   }
   return written;

Here's part of the "safe-read.c", that is part of GNU coreutils, which just checks for EINTR:

Code:

size_t
safe_rw (int fd, void const *buf, size_t count)
{
  for (;;)
    {
      ssize_t result = rw (fd, buf, count);


      if (0 <= result)
        return result;
      else if (IS_EINTR (errno))
        continue;
      else if (errno == EINVAL && SYS_BUFSIZE_MAX < count)
        count = SYS_BUFSIZE_MAX;
      else
        return result;
    }
}

Alright, how 'bout this then:

Code:

int rdpipe( pipe_t pipe, void *data, ssize_t *done )
{
	int ret;
	ssize_t byte = 0;
	char *dst = (void*)(&data), tmp[sizeof(void*)];
	
	while ( byte < sizeof(void*) )
	{
		int ret;
		ssize_t bytes = read( pipe, data, sizeof(void*) - byte );
		#prama message "Should consider using a mutex here as this is shared"
		ret = errno;
		
		for ( ssize_t b = 0; b < bytes; dst[byte] = tmp[b], ++b, ++byte );
		
		if ( ret != 0 && ret != EINTR )
		{
			*done = byte;
			return ret;
		}
	}
	
	*done = byte;
	return 0;
}

int wrpipe( pipe_t pipe, void *data, ssize_t *done )
{
	int ret;
	ssize_t byte = 0;
	char *dst = (void*)(&data), tmp[sizeof(void*)];
	
	while ( byte < sizeof(void*) )
	{
		int ret;
		ssize_t bytes = write( pipe, data, sizeof(void*) - byte );
		#prama message "Should consider using a mutex here as this is shared"
		ret = errno;
		
		for ( ssize_t b = 0; b < bytes; dst[byte] = tmp[b], ++b, ++byte );
		
		if ( ret != 0 && ret != EINTR )
		{
			*done = byte;
			return ret;
		}
	}
	
	*done = byte;
	return 0;
}

Which leads to code like this:

Code:

	ret = rdpipe( pipes[PIPE_RD], (void*)(&ptr), &bytes );
			
	if ( ret != 0 )
		continue;

Originally Posted by hamster_nz

I don't think 'errno' works the way this code expects...

Code:

        ssize_t bytes = write( pipe, data, sizeof(void*) - byte );
        #prama message "Should consider using a mutex here as this is shared"
        ret = errno;
         
        for ( ssize_t b = 0; b < bytes; dst[byte] = tmp[b], ++b, ++byte );
         
        if ( ret != 0 && ret != EINTR )

So if no error occurs on the write, errno most likely holds junk.

Ah, yeah that's an oversight, thx for pointing it out. Got a different problem that I don't understand atm, while I'm fixing that mind taking a look for me.

The error I'm getting:

Code:

workers.c:229:11: error: expected identifier before numeric constant
  229 |         , 64
      |           ^~

The line of code causing the error:

Code:

struct memory_group workerv = {0}, blockv = {0};
...
void *temp = inc_memory_group_total( struct shared_block, &blockv, 64 ); // This line is where the error pops up, I condensed it fot the post

Header file is small so I'll just dump it entirely (the macro above is at the bottom):

Code:

#ifndef MEMORY_H
#define MEMORY_H

#ifdef _WIN32
#include <windows.h>
#else
#include <unistd.h>
#include <stdint.h>
#include <stdbool.h>
#include <inttypes.h>
#include <malloc.h>
#endif

struct memory_block
{
	void *block;
	size_t bytes;
};

#define get_memory_block_bytes( block ) ((block)->bytes)
#define get_memory_block_block( T, block ) ((T)((block)->block))

/* Initialises memory block structure
 * 
 * @memory_block Pointer to structure you want to initialise, will segfault on
 * invalid pointer
 * 
 * @bytes Number of bytes you want to start with
 * 
 * Will attempt to allocate block of bytes to point the structure to
 * 
 * @return NULL on failure, valid pointer on success
*/
void * new_memory_block( struct memory_block *memory_block, size_t bytes );

/* Reallocates memory pointed by structure
 * 
 * @memory_block Pointer to structure you want to reallocate pointed memory of,
 * will segfault on invalid pointer
 * 
 * @bytes New number of bytes to allocate
 * 
 * Will attempt to reallocate block pointed to by structure and update the
 * members on success, on failure it is left as is, attempts with invalid
 * pointer in memory_block->block can lead to undefined behaviour, currently
 * whatever realloc() does in this situation is what happens here as it is
 * passed directly onto the function
 * 
 * @return NULL on failure, memory_block->block on success
*/
void * alt_memory_block( struct memory_block *memory_block, size_t bytes );

/* Reallocates memory pointed by structure
 * 
 * @memory_block Pointer to structure you want to reallocate pointed memory of,
 * will segfault on invalid pointer
 * 
 * @bytes New number of bytes to allocate
 * 
 * Will attempt to reallocate block pointed to by structure ONLY if the wanted
 * bytes are more than the current amount allocated and update the
 * members on success, on failure it is left as is, attempts with invalid
 * pointer in memory_block->block can lead to undefined behaviour, currently
 * whatever realloc() does in this situation is what happens here as it is
 * passed directly onto the function
 * 
 * @return NULL on failure, memory_block->block on success
*/
void * inc_memory_block( struct memory_block *memory_block, size_t bytes );

/* Reallocates memory pointed by structure
 * 
 * @memory_block Pointer to structure you want to reallocate pointed memory of,
 * will segfault on invalid pointer
 * 
 * @bytes New number of bytes to allocate
 * 
 * Will attempt to reallocate block pointed to by structure ONLY if the wanted
 * bytes are less than the current amount allocated and update the
 * members on success, on failure it is left as is, attempts with invalid
 * pointer in memory_block->block can lead to undefined behaviour, currently
 * whatever realloc() does in this situation is what happens here as it is
 * passed directly onto the function
 * 
 * @return NULL on failure, memory_block->block on success
*/
void * dec_memory_block( struct memory_block *memory_block, size_t bytes );

/* Releases pointed memory
 * 
 * @memory_block Pointer to memory block structure holding a pointer to the
 * block to be released, will segfault on invalid pointer
 * 
 * Will attempt to release memory pointed to by memory_block->block, afterwards
 * it will reinitialise the members to 0 & NULL, invalid pointers can lead to
 * undefined behaviour, currently whatever free() does in this situation is
 * what happens here as the pointer is directly passed on to the function
*/
void del_memory_block( struct memory_block *memory_block );

struct memory_group
{
	struct memory_block memory_block;
	int total, count, focus;
};

#define get_memory_group_total( group ) ((group)->total)
#define get_memory_group_count( group ) ((group)->count)
#define get_memory_group_focus( group ) ((group)->focus)
#define get_memory_group_bytes( group ) \
	get_memory_block_bytes( ((struct memory_block*)group) )
#define get_memory_group_group( T, group ) \
	get_memory_block_block( T*, ((struct memory_block*)group) )
#define get_memory_group_entry( T, group, index ) \
	(get_memory_group_group( T, group )[index])

#define new_memory_group_total( T, group, total ) \
	new_memory_block( ((struct memory_block*)group), sizeof(T) * total )
#define alt_memory_group_total( T, group, total ) \
	alt_memory_block( ((struct memory_block*)group), sizeof(T) * total )	
#define inc_memory_group_total( T, group, total ) \
	inc_memory_block( ((struct memory_block*)group), sizeof(T) * (((group)->total) + total) )
#define dec_memory_group_total( T, group, total ) \
	dec_memory_block( ((struct memory_block*)group), sizeof(T) * (((group)->total) - total) )
#define del_memory_group_total( group ) \
	del_memory_block( ((struct memory_block*)group) )

#endif

Never mind, the problem mysteriously solved itself when I started using the get_memory_group_total() macro instead, perhaps the macro handler hadn't placed the correct name at that point?

New problem of which I can't fathom how it occurs, an array of pointers to the worker objects given to each worker is not giving a pointer upon the read of an element that was set directly prior to the overarching loop, at no point do I set the pointer to NULL until the worker reports it's
death, there should be a pointer there at the point the overarching loop goes looking for it to send a wait/continue message, here's the overarching loop from the point of the pointer being set (worker_group.count starts is 0 prior to this point):

Code:

	workers_group.count++;
	workers[workers_group.count] = worker;
	
	if ( _new_worker( 1, worker, pipes, run ) != 0 )
	{
		puts("Failed to create worker thread, exiting..." );
		goto cleanup;
	}
	
	puts("Starting main worker loop");
	
	while ( workers_group.count > 0 && (ready = poll_pipe( pipes[PIPE_RD] )) >= 0 )
	{
		ssize_t bytes;
		struct worker_msg *worker_msg = NULL, own_msg = {0};
		void *_own = &own_msg, *own = (void*)(&_own)
			, *_ptr = NULL, *ptr = (void*)&(_ptr);
			
		if ( ready != 1 )
			continue;
			
		ret = rdpipe( pipes[PIPE_RD], ptr, &bytes );
		
		if ( ret != 0 )
		{
			printf( "Error 0x%08X (%d) '%s'\n", ret, ret, strerror(ret) );
			continue;
		}
		
		puts("Main worker is reading from pointer");
		
		worker_msg = _ptr;
		
		switch ( worker_msg->type )
		{
		case WORKER_MSG_DIED:
			puts("Main worker is removing a worker");
			workers_group.count--;
			worker = worker_msg->data;
			worker->create_thread_ret = ENOMEDIUM;
			workers[worker->num] = NULL;
			_del_worker( worker );
			free( worker );
			break;
		case WORKER_MSG_ALLOC:
		{
			struct worker_block *worker_block = worker_msg->data;
			struct memory_block *memory_block = worker_block->memory_block;
			
			if ( worker_block->worker )
			{
				worker_msg->type = WORKER_MSG_WAIT;
				
				for ( int w = 1; w < workers_group.total; ++w )
				{
					worker = workers[w];
					if ( worker )
					{
						puts("Main worker is sending wait message");
						ret = wrpipe( worker->own_pipes[PIPE_WR], ptr, &bytes );
						
						#pragma message "Best to revisit this later"
						if ( ret != 0 )
						{
							wrpipe_panic:
							printf
							(
								"Could not recover from "
								"error 0x%08X (%d) '%s', exiting..."
								, ret, ret, strerror(ret)
							);
							exit(EXIT_FAILURE);
						}
						
						printf("Main worker is waiting on a response from "
								"worker %d", worker->num );
						
						while ( (ready = poll_pipe( pipes[PIPE_RD] )) >= 0 )
						{
							puts("Main worker is checking if pipe is ready");
							
							if ( ready == 1 )
							{
								ret = rdpipe( pipes[PIPE_RD], ptr, &bytes );
								
								if ( ret != 0 )
									break;
								
								if ( worker_msg->type != WORKER_MSG_CONT || worker_msg->worker != worker )
								{
									puts("Main worker is sending resend message");
									
									own_msg.type = WORKER_MSG_RESEND;
									own_msg.worker = NULL;
									own_msg.data = worker_msg;
									ret = wrpipe( worker_msg->worker->own_pipes[PIPE_WR], own, &bytes );
									
									if ( ret != 0 )
										goto wrpipe_panic;
								}
								else
									break;
							}
						}
					}
				}
				
				puts("Main worker is performing allocation");
				
				if ( worker_block->want )
				{
					int i = 0;
					
					for ( ; i < blocks_group.total; ++i )
					{
						block = &blocks[i];
						if ( block->block == memory_block->block )
						{
							block->holders++;
							break;
						}
					}
					
					if ( i == blocks_group.total )
					{
						for ( i = 0; i < blocks_group.total; ++i )
						{
							block = &blocks[i];
							if ( !(block->block) )
							{
								block->holders++;
								block->block = memory_block->block;
								blocks_group.count++;
								break;
							}
						}
					}
					
					if ( i == blocks_group.total )
					{
						void *temp;
						temp = inc_memory_group_total
							( struct shared_block, &blocks_group, 64 );
						
						if ( temp )
						{
							blocks = temp;
							block = &blocks[i];
							block->holders++;
						}
					}
					
					if ( i < blocks_group.total )
					{
						inc_memory_block( memory_block, worker_block->want );
						block->block = memory_block->block;
					}
				}
				else
				{
					for ( int i = 0; i < blocks_group.total; ++i )
					{
						block = &blocks[i];
						if ( block->block == memory_block->block )
						{
							block->holders--;
							if ( !block->holders )
							{
								del_memory_block( memory_block );
								block->block = NULL;
								blocks_group.count--;
								break;
							}
						}
					}
				}
			}
			else if ( worker_block->want )
				alt_memory_block( memory_block, worker_block->want );
			else
				del_memory_block( memory_block );
			
			worker_msg->type = WORKER_MSG_CONT;
			
			for ( int w = 0; w < workers_group.total; ++w )
			{
				worker = workers[w];
				if ( worker )
				{
					printf("Main worker is sending continue message to "
						"worker %d", worker->num );
					
					ret = wrpipe( worker->own_pipes[PIPE_WR], ptr, &bytes );
					
					if ( ret != 0 )
						goto wrpipe_panic;
				}
			}
			
			break;
		}
		}
	}

Anyone able to figure it out?

Managed to resolve the issue and add in the debug target along with gede (although work needs to be done for cross compiler compatibility), the details are here:
GitHub - awsdert/gasp at 89b7e10e6377f857ae8607d1a54f062661af9884

I've hit what I think might be my final worker related problem, a plain segfault when there should be none:

Code:

int seek_worker_msg( struct worker *worker, int msg )
{
	int ret;
	ssize_t bytes;
	void *ptr = worker->ptr2ptr2src_msg;
	
	worker->ptr2src_msg = NULL;
	
	printf("Worker %d is seeking a message\n", worker->num );
	
	while ( (ret = poll_pipe( worker->own_pipes[PIPE_RD] )) >= 0 )
	{
		printf("Worker %d is checking if pipe is ready\n", worker->num );
		
		if ( ret == 1 )
		{
			struct worker_msg *own_msg = worker->ptr2own_msg, *src_msg;
			
			printf("Worker %d is reading a message pointer\n", worker->num );
			
			ret = rdpipe( worker->own_pipes[PIPE_RD], ptr, &bytes );
			
			if ( ret != 0 )
				return ret;
			
			src_msg = worker->ptr2src_msg;
			
			printf("Worker %d is checking for message type\n", worker->num );
			
			// This line here
			if ( src_msg->type == WORKER_MSG_WAIT )
			{
				send_worker_msg( worker, WORKER_MSG_CONT, own_msg->data );
				continue;
			}
			
			if ( msg >= 0 && src_msg->type != msg )
				continue;
			
			break;
		}
	}
	
	printf("Worker %d successfully sought a message\n", worker->num );
	
	return 0;
}

Gotta leave for work soon so what ever suggestions you have I will not be trying them until late tonight.

Never mind, just found the source of the issue, during initialisation of the worker object I mis-initialised the pointers to the pointers