[Squashfs-devel] bug? squashfs3.1-r2/squashfs-tools

[<hoo...@ya...>]

Hello Phillip and all,

I have found some bugs in squashfs3.1-r2/squashfs-tools. Will you check
and correct them?

- compile option to support linuxthread
  need to define -D_REENTRANT to all of source files of mksquashfs.

- non-reentrant libraies
  Some of library functions are not thread-safe. The typical one is
  malloc family. It will update a global variable in a process. In
  mksquashfs, all of malloc() calls after initialise_threads() should
  use pthread_mutex_t or something.

  Ex.
  pthread_mutex_t mem_mtx = PTHREAD_MUTEX_INITIALIZER;
  void *safe_malloc(size_t sz)
  {
	void *p;
	pthread_mutex_lock(&mem_mtx);
	p = malloc(sz);
	pthread_mutex_unlock(&mem_mtx);
	return p;
  }
  void safe_free(void *p)
  {
	pthread_mutex_lock(&mem_mtx);
	free(p);
	pthread_mutex_unlock(&mem_mtx);
  }
  Also for calloc(), realloc() and strdup().

  Additionally, zlib may call malloc() and free(). So mksquashfs should
  set both of z_stream.zalloc and .zfree members.
  I am afraid setting .zalloc and .zfree may not help uncompress() in
  zlib, and mksquashfs may need to acquire the same lock for
  safe_malloc().

- global variables versus signal handler and thread
  Generally global variables which are used in a signal hander or a
  thread, should be declared as volatile or should be protected by mutex
  lock or something.
  Some of them in mksquashfs are less important at this meaning, because
  they are set in main() once and never be changed. But the others may
  be updated in parallel.

- statements hard to read
  These statements confused me.

	if(file_buffer->fragment = count == frag_block)
	compressed_data = inode_dir_offset + inode_dir_file_size & ~(SQUASHFS_METADATA_SIZE - 1);
	uncompressed_data = inode_dir_offset + inode_dir_file_size & (SQUASHFS_METADATA_SIZE - 1);
	if(root_process = geteuid() == 0)

  Do these inserted parentheses follow what you want to do?

	if((file_buffer->fragment = (count == frag_block)))
	compressed_data = (inode_dir_offset + inode_dir_file_size)
		& ~(SQUASHFS_METADATA_SIZE - 1);
	uncompressed_data = (inode_dir_offset + inode_dir_file_size)
		& (SQUASHFS_METADATA_SIZE - 1);
	if((root_process = (geteuid() == 0)))

- global var 'swap' in read_fs.
  Should it be declared as 'extern'?


Thanks in advance.
Junjiro Okajima