On Thu, Dec 10, 2020 at 05:12:09PM -0700, Andreas Dilger wrote:
> > @@ -329,12 +369,20 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
> > }
> > if (!bitmap_tail_verify((unsigned char *) block_bitmap,
> > block_nbytes, fs->blocksize - 1))
> > - tail_flags |= EXT2_FLAG_BBITMAP_TAIL_PROBLEM;
> > + *tail_flags |= EXT2_FLAG_BBITMAP_TAIL_PROBLEM;
> > } else
> > memset(block_bitmap, 0, block_nbytes);
> > cnt = block_nbytes << 3;
> > +#ifdef HAVE_PTHREAD
> > + if (mutex)
> > + pthread_mutex_lock(mutex);
> > +#endif
> > retval = ext2fs_set_block_bitmap_range2(fs->block_map,
> > blk_itr, cnt, block_bitmap);
> > +#ifdef HAVE_PTHREAD
> > + if (mutex)
> > + pthread_mutex_unlock(mutex);
> > +#endif
>
> (style) It wouldn't be terrible to have wrappers around these functions
> instead of inline #ifdef in the few places they are used, like:
>
> #ifdef HAVE_PTHREAD
> static void unix_pthread_mutex_lock(pthread_mutex_t *mutex)
> {
> if (mutex)
> pthread_mutex_lock(mutex);
> }
> static void unix_pthread_mutex_unlock(pthread_mutex_t *mutex)
> {
> if (mutex)
> pthread_mutex_unlock(mutex);
> }
> #else
> #define unix_pthread_mutex_lock(mutex) do {} while (0)
> #define unix_pthread_mutex_unlock(mutex) do {} while (0)
> #endif
We'd also need to have a typedef for mutex_t which is either
pthreads_mutex_t if pthreads are available, or an int (or some other
placeholder type) if it isn't.
I had tried to make sure that rw_bitmaps.c will correctly compile with
HAVE_PTHREAD and HAVE_PTHREAD_H are undefined. It looks like I didn't
quite get it completely working since there's at leasts one function
signature where we have an unprotected use of pthread_mutex_t, so
that's something we should check before finalizing the patch --- in
addition to the unprotected use of pthread_mutex_{lock,unlock} that
you pointed out.
- Ted
