Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/fixing clang gnu compiler warnings #45

Merged
merged 15 commits into from
May 16, 2023
Merged

Feature/fixing clang gnu compiler warnings #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
06bb5a6
Support for CMake builds. Added Linux stub port for now to allow cross
phelter Sep 30, 2022
321db9e
Fixing misuse of doxygen documentation
phelter Oct 23, 2022
b583a73
Converting prints from %lu to %u where possible as per request in com…
phelter Nov 1, 2022
15c8f65
Fixing clang compiler warnings.
phelter Nov 1, 2022
bdc2f5a
Fixing build test to work regardless of configuration.
phelter Nov 3, 2022
c095b29
Adding in for Zynq port dependency on uncached_memory target.
phelter Nov 3, 2022
c6e8021
Excluding build_test from all build - must request it explicitly.
phelter Nov 3, 2022
5163366
Ensuring XPAR_XSDPS_0_IS_CACHE_COHERENT is 1 for this build since using
phelter Nov 3, 2022
105f900
Updating compiler warnings.
phelter Nov 3, 2022
a37b5f0
Fixing documentation, signed/unsigned conversion. Bugfix for time bas…
phelter Nov 4, 2022
8ff184c
Merge branch 'master' into feature/fixing-clang-gnu-compiler-warnings
phelter May 1, 2023
3f8fb14
Fixing error creation using FF_createERR everywhere missed.
phelter May 1, 2023
4aa9dbb
Fixing build_test in ci.yml
phelter May 1, 2023
325a308
Merge branch 'main' into feature/fixing-clang-gnu-compiler-warnings
amazonKamath May 11, 2023
d500500
Uncrustify: triggered by comment.
actions-user May 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ jobs:
- uses: actions/checkout@v2
- name: Build checks (Default configuration)
run: |
cmake -S . -B build/ -DFREERTOS_PLUS_FAT_TEST_CONFIGURATION=DEFAULT_CONF
make -C build/
cmake -S . -B build -DFREERTOS_PLUS_FAT_TEST_CONFIGURATION=DEFAULT_CONF
cmake --build build --target freertos_plus_fat_build_test

# complexity:
# runs-on: ubuntu-latest
Expand Down
10 changes: 2 additions & 8 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,22 +186,16 @@ target_compile_definitions( freertos_plus_fat

target_compile_options( freertos_plus_fat
PRIVATE
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-array-bounds>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is compiling with clang 14.0

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I feel array-bounds violations are fairly useful warnings, but I'm unaware whether there were intended "false positive" types being reported. Regardless plenty of other warnings were fixed, so approved first round of reviews.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built POSIX test on Ubuntu 22.04 with GCC11 with Warray-bounds and no array bound warnings came up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dachalco I also use GCC11.3.0 and had to compile with that added to ensure no errors. I treat all warnings as errors. So this list is merely stating what is detected by Clang and GNU when compiling with those.

If they are no longer there then feel free to delete them, but the process I typically use is:

  1. identify everything first the linters/compilers show up
  2. Remove the warnings I intend to fix,
  3. Fix the warnings until those compile without error.

If you have a build that confirms this is no longer there then feel free to remove once the workflow adds those.

$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-qual>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-constant-conversion>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-covered-switch-default>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-documentation-unknown-command>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-documentation>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-extra-semi-stmt>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-declaration-after-statement>
$<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wno-format>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-implicit-int-conversion>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-overflow>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-pedantic>

$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-reserved-macro-identifier>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-shorten-64-to-32>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-tautological-constant-out-of-range-compare>
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-type-limits>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-undef>
Expand Down
146 changes: 68 additions & 78 deletions ff_dir.c
View file
Open in desktop

Large diffs are not rendered by default.

67 changes: 30 additions & 37 deletions ff_fat.c
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ FF_Buffer_t * prvGetFromFATBuffers( FF_IOManager_t * pxIOManager,
{
/* Setting an error code without the Module/Function,
* will be filled-in by the caller. */
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_ERRFLAG );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_ERRFLAG );
}
}

Expand Down Expand Up @@ -372,7 +372,7 @@ FF_Buffer_t * prvGetFromFATBuffers( FF_IOManager_t * pxIOManager,

if( FF_isERR( xError ) )
{
xError = FF_GETERROR( xError ) | FF_GETFATENTRY;
xError = FF_CreateError( FF_GETERROR( xError ), FF_GETFATENTRY );
}
else
{
Expand All @@ -394,7 +394,7 @@ FF_Buffer_t * prvGetFromFATBuffers( FF_IOManager_t * pxIOManager,

if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_GETFATENTRY );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_GETFATENTRY );
}
else
{
Expand Down Expand Up @@ -444,7 +444,7 @@ uint32_t FF_getFATEntry( FF_IOManager_t * pxIOManager,
{
/* _HT_ find a more specific error code.
* Probably not really important as this is a function internal to the library. */
xError = ( FF_Error_t ) ( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE | FF_GETFATENTRY );
xError = FF_createERR( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE, FF_GETFATENTRY );
}
else
{
Expand Down Expand Up @@ -505,7 +505,7 @@ uint32_t FF_getFATEntry( FF_IOManager_t * pxIOManager,

if( FF_isERR( xError ) )
{
xError = FF_GETERROR( xError ) | FF_GETFATENTRY;
xError = FF_createERR( FF_GETERROR( xError ), FF_GETFATENTRY );
}
else
{
Expand Down Expand Up @@ -568,7 +568,7 @@ uint32_t FF_getFATEntry( FF_IOManager_t * pxIOManager,
*pxError = xError;
}

return ( int32_t ) ulFATEntry;
return ulFATEntry;
} /* FF_getFATEntry() */
/*-----------------------------------------------------------*/

Expand All @@ -595,14 +595,14 @@ FF_Error_t FF_ClearCluster( FF_IOManager_t * pxIOManager,

if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_CLEARCLUSTER );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_CLEARCLUSTER );
break;
}

memset( pxBuffer->pucBuffer, 0x00, pxIOManager->usSectorSize );
}

xError = FF_BlockWrite( pxIOManager, ulBaseLBA + xIndex, 1, pxBuffer->pucBuffer, pdFALSE );
xError = FF_BlockWrite( pxIOManager, ( uint32_t ) ( ulBaseLBA + xIndex ), 1U, pxBuffer->pucBuffer, pdFALSE );

if( FF_isERR( xError ) )
{
Expand Down Expand Up @@ -631,15 +631,13 @@ FF_Error_t FF_ClearCluster( FF_IOManager_t * pxIOManager,
/*-----------------------------------------------------------*/

/**
* @private
* @brief Returns the Cluster address of the Cluster number from the beginning of a chain.
*
* @param pxIOManager FF_IOManager_t Object
* @param ulStart Cluster address of the first cluster in the chain.
* @param ulCount Number of Cluster in the chain,
*
*
*
**/
uint32_t FF_TraverseFAT( FF_IOManager_t * pxIOManager,
uint32_t ulStart,
Expand Down Expand Up @@ -728,7 +726,6 @@ uint32_t FF_FindEndOfChain( FF_IOManager_t * pxIOManager,
/*-----------------------------------------------------------*/

/**
* @private
* @brief Tests if the ulFATEntry is an End of Chain Marker.
*
* @param pxIOManager FF_IOManager_t Object
Expand Down Expand Up @@ -826,7 +823,7 @@ BaseType_t FF_isEndOfChain( FF_IOManager_t * pxIOManager,
{
if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_PUTFATENTRY );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_PUTFATENTRY );
break;
}

Expand All @@ -844,7 +841,7 @@ BaseType_t FF_isEndOfChain( FF_IOManager_t * pxIOManager,
{
if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_PUTFATENTRY );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_PUTFATENTRY );
break;
}

Expand All @@ -864,7 +861,6 @@ BaseType_t FF_isEndOfChain( FF_IOManager_t * pxIOManager,
#endif /* if ( ffconfigFAT12_SUPPORT != 0 ) */

/**
* @private
* @brief Writes a new Entry to the FAT Tables.
*
* @param pxIOManager IOMAN object.
Expand Down Expand Up @@ -899,7 +895,7 @@ FF_Error_t FF_putFATEntry( FF_IOManager_t * pxIOManager,
if( ( ulCluster == 0ul ) || ( ulCluster >= pxIOManager->xPartition.ulNumClusters ) )
{
/* find a more specific error code. */
xError = ( FF_Error_t ) ( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE | FF_PUTFATENTRY );
xError = FF_createERR( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE, FF_PUTFATENTRY );
}
else
{
Expand Down Expand Up @@ -950,7 +946,7 @@ FF_Error_t FF_putFATEntry( FF_IOManager_t * pxIOManager,

if( FF_isERR( xError ) )
{
xError = FF_GETERROR( xError ) | FF_PUTFATENTRY;
xError = FF_createERR( FF_GETERROR( xError ), FF_PUTFATENTRY );
break;
}

Expand Down Expand Up @@ -1007,13 +1003,12 @@ FF_Error_t FF_putFATEntry( FF_IOManager_t * pxIOManager,
/*-----------------------------------------------------------*/

/**
* @private
* @brief Finds a Free Cluster and returns its number.
*
* @param pxIOManager IOMAN Object.
*
* @return The number of the cluster found to be free.
* @return 0 on error.
* @retval 0 on error.
**/
#if ( ffconfigFAT12_SUPPORT != 0 )
static uint32_t prvFindFreeClusterSimple( FF_IOManager_t * pxIOManager,
Expand Down Expand Up @@ -1060,7 +1055,7 @@ FF_Error_t FF_putFATEntry( FF_IOManager_t * pxIOManager,
{
/* There is no free cluster any more. */
ulCluster = 0;
xError = ( FF_Error_t ) ( FF_FINDFREECLUSTER | FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE );
xError = FF_createERR( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE, FF_FINDFREECLUSTER );
}

*pxError = xError;
Expand Down Expand Up @@ -1112,7 +1107,7 @@ uint32_t FF_FindFreeCluster( FF_IOManager_t * pxIOManager,

if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_FINDFREECLUSTER );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_FINDFREECLUSTER );
}
else
{
Expand All @@ -1134,8 +1129,8 @@ uint32_t FF_FindFreeCluster( FF_IOManager_t * pxIOManager,
uint32_t ulFATSector;
uint32_t ulFATOffset;

ulEntriesPerSector = pxIOManager->usSectorSize / xEntrySize;
ulFATOffset = ulCluster * xEntrySize;
ulEntriesPerSector = ( uint32_t ) ( pxIOManager->usSectorSize / xEntrySize );
ulFATOffset = ( uint32_t ) ( ulCluster * xEntrySize );

/* Start from a sector where the first free entry is expected,
* and iterate through every FAT sector. */
Expand All @@ -1147,7 +1142,7 @@ uint32_t FF_FindFreeCluster( FF_IOManager_t * pxIOManager,

if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_FINDFREECLUSTER );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_FINDFREECLUSTER );
break;
}

Expand All @@ -1156,7 +1151,7 @@ uint32_t FF_FindFreeCluster( FF_IOManager_t * pxIOManager,
/* Double-check: don't use non-existing clusters */
if( ulCluster >= uNumClusters )
{
xError = ( FF_Error_t ) ( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE | FF_FINDFREECLUSTER );
xError = FF_createERR( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE, FF_FINDFREECLUSTER );
break;
}

Expand Down Expand Up @@ -1201,7 +1196,7 @@ uint32_t FF_FindFreeCluster( FF_IOManager_t * pxIOManager,
if( ( FF_isERR( xError ) == pdFALSE ) &&
( ulFATSector == pxIOManager->xPartition.ulSectorsPerFAT ) )
{
xError = ( FF_Error_t ) ( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE | FF_FINDFREECLUSTER );
xError = FF_createERR( FF_ERR_IOMAN_NOT_ENOUGH_FREE_SPACE, FF_FINDFREECLUSTER );
}
} /* if( FF_isERR( xError ) == pdFALSE ) */
} /* if( pxIOManager->xPartition.ucType != FF_T_FAT12 ) */
Expand Down Expand Up @@ -1243,10 +1238,9 @@ uint32_t FF_FindFreeCluster( FF_IOManager_t * pxIOManager,
/*-----------------------------------------------------------*/

/**
* @private
* @brief Creates a Cluster Chain
* @return > 0 New created cluster
* @return = 0 See pxError
* @retval > 0 New created cluster
* @retval = 0 See pxError
**/
uint32_t FF_CreateClusterChain( FF_IOManager_t * pxIOManager,
FF_Error_t * pxError )
Expand Down Expand Up @@ -1316,17 +1310,16 @@ uint32_t FF_GetChainLength( FF_IOManager_t * pxIOManager,
/*-----------------------------------------------------------*/

/**
* @private
* @brief Free's Disk space by freeing unused links on Cluster Chains
*
* @param pxIOManager, IOMAN object.
* @param pxIOManager IOMAN object.
* @param ulStartCluster Cluster Number that starts the chain.
* @param ulCount Number of Clusters from the end of the chain to unlink.
* @param ulCount 0 Means Free the entire chain (delete file).
* @param ulCount 1 Means mark the start cluster with EOF.
* @param xDoTruncate Perform truncation of the FAT entry
* when non-zero - truncate
* when 0 - do not perform truncation
*
* @return 0 On Success.
* @return -1 If the device driver failed to provide access.
* @retval 0 On Success.
* @retval -1 If the device driver failed to provide access.
*
**/
FF_Error_t FF_UnlinkClusterChain( FF_IOManager_t * pxIOManager,
Expand Down Expand Up @@ -1494,7 +1487,7 @@ uint32_t FF_CountFreeClusters( FF_IOManager_t * pxIOManager,

if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_COUNTFREECLUSTERS );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_COUNTFREECLUSTERS );
}
else
{
Expand Down Expand Up @@ -1542,7 +1535,7 @@ uint32_t FF_CountFreeClusters( FF_IOManager_t * pxIOManager,

if( pxBuffer == NULL )
{
xError = ( FF_Error_t ) ( FF_ERR_DEVICE_DRIVER_FAILED | FF_COUNTFREECLUSTERS );
xError = FF_createERR( FF_ERR_DEVICE_DRIVER_FAILED, FF_COUNTFREECLUSTERS );
break;
}

Expand Down
Loading