Changeset View
Standalone View
usr.sbin/fstyp/befs.c
- This file was added.
/*- | |||||
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD | |||||
* | |||||
* Copyright (c) 2021 Miguel Gocobachi | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND | |||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | |||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE | |||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | |||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | |||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
* | |||||
*/ | |||||
pfg: This line is not needed, or accurate., since you are not Berkeley. | |||||
#include <sys/cdefs.h> | |||||
__FBSDID("$FreeBSD$"); | |||||
#include <stdint.h> | |||||
#include <stdio.h> | |||||
#include <stdlib.h> | |||||
#include <string.h> | |||||
#include "fstyp.h" | |||||
#define B_OS_NAME_LENGTH 32 | |||||
#define BEFS_BLOCK_OFFSET 512 | |||||
#define BEFS_SUPER_BLOCK_MAGIC1 0x42465331 | |||||
Done Inline ActionsSince this file is befs, but these symbols are BFS, perhaps add a comment explaining why they are BFS instead of BEFS? rpokala: Since this file is `befs`, but these symbols are `BFS`, perhaps add a comment explaining why… | |||||
Done Inline ActionsShould we cast this constant expression to magic1's type int32_t? (Should the type be int32_t in any case?) pstef: Should we cast this constant expression to magic1's type int32_t? (Should the type be int32_t… | |||||
Done Inline ActionsThe API documentation is defined this way, and every other system that implemented it are using this way as constant and only defining the int32_t at the structs. miguel_gocobachi.dev: The API documentation is defined this way, and every other system that implemented it are using… | |||||
struct disk_super_block { | |||||
char name[B_OS_NAME_LENGTH]; | |||||
int32_t magic1; | |||||
}; | |||||
int | |||||
fstyp_befs(FILE *fp, char *label, size_t size) | |||||
{ | |||||
struct disk_super_block *volume; | |||||
volume = read_buf(fp, BEFS_BLOCK_OFFSET, sizeof(*volume)); | |||||
Done Inline Actionswonder if we should provide a define (e.g. BFS_SUPER_BLOCK_OFFSET) for 512. yuripv: wonder if we should provide a define (e.g. `BFS_SUPER_BLOCK_OFFSET`) for `512`. | |||||
if (volume == NULL) { | |||||
return (1); | |||||
} | |||||
if (volume->magic1 == BEFS_SUPER_BLOCK_MAGIC1) { | |||||
strlcpy(label, volume->name, size); | |||||
free(volume); | |||||
Done Inline Actionswhy bzero() the buffer if we are going to overwrite with nul-terminated string anyway? yuripv: why bzero() the buffer if we are going to overwrite with nul-terminated string anyway? | |||||
Done Inline Actionsno magic is needed for the 3rd argument, it should be exactly the size of provided buffer yuripv: no magic is needed for the 3rd argument, it should be exactly the `size` of provided buffer | |||||
Done Inline Actionsoh, we can't expect volume->name to be nul-terminated, then it should be MIN(size, BFS_OS_NAME_LENGTH + 1) so that we copy all of it and null-terminate? yuripv: oh, we can't expect `volume->name` to be nul-terminated, then it should be `MIN(size… | |||||
Done Inline ActionsI want it to be safe, but then using this way I don't really need to rtrim(), right? miguel_gocobachi.dev: I want it to be safe, but then using this way I don't really need to rtrim(), right? | |||||
return (0); | |||||
} | |||||
Done Inline ActionsToo many white lines in this function, IMHO. A blank line between declarations and code is enough. pfg: Too many white lines in this function, IMHO. A blank line between declarations and code is… | |||||
free(volume); | |||||
return (1); | |||||
} |
This line is not needed, or accurate., since you are not Berkeley.