Differential D29917 Diff 93828 usr.sbin/fstyp/befs.c

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.
				*
				*/

				pfgUnsubmitted Done Inline Actions This line is not needed, or accurate., since you are not Berkeley. 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
				rpokalaUnsubmitted Done Inline Actions Since 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…

				pstefUnsubmitted Done Inline Actions Should we cast this constant expression to magic1's type int32_t? (Should the type be int32_t in any case?) This is not a request for change. pstef: Should we cast this constant expression to magic1's type int32_t? (Should the type be int32_t…
				miguel_gocobachi.devAuthorUnsubmitted Done Inline Actions The 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));

				yuripvUnsubmitted Done Inline Actions wonder 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);
				yuripvUnsubmitted Done Inline Actions why 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?

				yuripvUnsubmitted Done Inline Actions no 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
				yuripvUnsubmitted Done Inline Actions oh, 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…
				miguel_gocobachi.devAuthorUnsubmitted Done Inline Actions I 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);
				}
				pfgUnsubmitted Done Inline Actions Too 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);
				}