Hi William, the race condition possibility is a good point. Thanks for suggesting me. Will send a new version with using `rm -df` instead and add the appropriate CC lists. Thank you! On 7/10/23 08:07, William Breathitt Gray wrote: > On Fri, Jul 07, 2023 at 09:08:35PM +0700, Anh Tuan Phan wrote: >> rmdir requires the directory exist so it causes "make -C tools clean" >> failed if someone only builds other tools but not counter. This commit >> adds checking the directory exist before doing rmdir. >> >> Signed-off-by: Anh Tuan Phan <tuananhlfc@xxxxxxxxx> >> --- >> tools/counter/Makefile | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tools/counter/Makefile b/tools/counter/Makefile >> index a0f4cab71fe5..2907f3b3094b 100644 >> --- a/tools/counter/Makefile >> +++ b/tools/counter/Makefile >> @@ -40,7 +40,9 @@ $(OUTPUT)counter_example: $(COUNTER_EXAMPLE) >> clean: >> rm -f $(ALL_PROGRAMS) >> rm -rf $(OUTPUT)include/linux/counter.h >> - rmdir -p $(OUTPUT)include/linux >> + @if [ -d $(OUTPUT)include/linux ]; then \ >> + rmdir -p $(OUTPUT)include/linux; \ >> + fi >> find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.d' -delete >> >> install: $(ALL_PROGRAMS) >> -- >> 2.34.1 > > Hi Anh, > > Please CC <linux-iio@xxxxxxxxxxxxxxx> and <linux-kernel@xxxxxxxxxxxxxxx> > as well in the future so Counter users and developers can become aware > of relevant patches. > > One worry I have with this approach is the possible race condition where > the check for existence succeeds but the directory is deleted by another > agent before our rmdir executes. However, I'm not sure how we could > achieve such behavior atomically to prevent the issue. > > One alternative I've considered is perhaps a single find command to > search for and delete empty directories: > > find $(or $(OUTPUT),.) -type d -empty -delete > > But this will delete directories not created by the makefile which I > consider an unexpected behavior for the user (or at least very rude of > the script to do). > > Perhaps we should delete the directory tree explicitly: > > rm -df $(OUTPUT)include/linux > rm -df $(OUTPUT)include > > Although we lose the symmetry of rmdir to our previous mkdir, this > should prevent the race condition issue and succeed whether the > directories still exist or not. > > William Breathitt Gray
