[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Sep 4 10:46:58 UTC 2018



On 09/04/2018 10:52 AM, Alexander Graf wrote:
> 
> 
> On 04.09.18 09:49, AKASHI Takahiro wrote:
>> The whole content of include/fat.h is private to FAT implementation
>> and then should be guarded with CONFIG_FS_FAT.

Hello Takahiro,

doesn't this imply that building common/spl/spl_sata.c without FAT will
fail for CONFIG_SPL_SATA_SUPPORT=y (e.g. cm_t54_defconfig with FAT
disabled).

Did you run Travis CI on your patch series?

>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>> ---
>>  include/fat.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/fat.h b/include/fat.h
>> index 09e142368585..c02839dcb040 100644
>> --- a/include/fat.h
>> +++ b/include/fat.h
>> @@ -9,6 +9,8 @@
>>  #ifndef _FAT_H_
>>  #define _FAT_H_
>>  
>> +#ifdef CONFIG_FS_FAT
> 
> Isn't this missing an include of at least common.h to actually propagate
> the config define?
> 
> Also, you want to use #if CONFIG_IS_ENABLED(...) here to guard against
> SPL builds too.

Probably not:
common/spl/spl_fat.c:14:#include <fat.h>

Best regards

Heinrich Schuchardt

> 
> However, I don't fully grasp why you need this patch. Please describe in
> your commit message what the incentive is for creating it. What errors
> are you trying to protect against?
> 
> 
> Alex
> 
>> +
>>  #include <asm/byteorder.h>
>>  #include <fs.h>
>>  
>> @@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
>>  int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
>>  void fat_closedir(struct fs_dir_stream *dirs);
>>  void fat_close(void);
>> +#endif /* CONFIG_FS_FAT */
>>  #endif /* _FAT_H_ */
>>
> 


More information about the U-Boot mailing list