Page MenuHomeFreeBSD
Differential D43602

nvme: Use the NVMEM macro instead of expanded versions
ClosedPublic
Actions

Authored by jhb on Jan 25 2024, 10:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 5, 9:21 PM
Unknown Object (File)
Feb 10 2025, 3:53 AM
Unknown Object (File)
Feb 10 2025, 3:53 AM
Unknown Object (File)
Feb 10 2025, 3:53 AM
Unknown Object (File)
Jan 27 2025, 2:58 PM
Unknown Object (File)
Jan 24 2025, 5:06 PM
Unknown Object (File)
Jan 23 2025, 6:47 PM
Unknown Object (File)
Jan 22 2025, 2:53 AM
View All 47 Files
Subscribers
dab

Details

Reviewers
imp
chuck
Commits
rG2208f2c15688: nvme: Use the NVMEM macro instead of expanded versions
rG8488fc417fc2: nvme: Use the NVMEM macro instead of expanded versions
Summary

A few of these omitted a shift of 0, but this is more consistent.

Sponsored by: Chelsio Communications

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 55616
Build 52505: arc lint + arc unit

Revision Contents
Changeset List

PathSize
sys/
dev/
nvme/
6 lines
2 lines

Diff 133366

View Options

sys/dev/nvme/nvme_ctrlr.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2023 Dave Cottlehuber <dch@FreeBSD.org>
*
* 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.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <errno.h>
#include <sys/param.h>
#include <stdio.h>
#include <string.h>
#include <fetch.h>
#include <lua.h>
#include "lauxlib.h"
#include "lfetch.h"
/*
* Minimal implementation of libfetch
*/
static int
lfetch_parse_url(lua_State *L)
{
const char *url = luaL_checkstring(L, 1);
struct url *u;
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
lua_newtable(L);
lua_pushstring(L, u->scheme);
lua_setfield(L, -2, "scheme");
lua_pushstring(L, u->user);
lua_setfield(L, -2, "user");
lua_pushstring(L, u->pwd);
lua_setfield(L, -2, "password");
lua_pushstring(L, u->host);
lua_setfield(L, -2, "host");
/* if port is not explicitly set, it defaults to 0, not scheme */
kevansUnsubmitted
Done
Inline Actions

My gut reaction to this is that we probably shouldn't expose the port at all if it's not explicitly set, just leave it nil in this table to allow for more idiomatic usage where you can simply test the port as truthy before using it.f

kevans: My gut reaction to this is that we probably shouldn't expose the port at all if it's not…
lua_pushinteger(L, u->port);
lua_setfield(L, -2, "port");
/* doc is also known as the query string for http(s) */
lua_pushstring(L, u->doc);
lua_setfield(L, -2, "doc");
fetchFreeURL(u);
return (1);
}
static int
lfetch_get_url(lua_State *L) {
kevansUnsubmitted
Done
Inline Actions

Brace on its own line

kevans: Brace on its own line
const char *url = luaL_checkstring(L, 1);
const char *out = luaL_checkstring(L, 2);
struct url *u;
kevansUnsubmitted
Done
Inline Actions

This should move up above the blank line

kevans: This should move up above the blank line
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
FILE *file = fopen(out, "wb");
kevansUnsubmitted
Done
Inline Actions

IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of this function.

kevans: IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of…
if (file == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to open output file: %s", strerror(errno));
return 2;
}
FILE *fetch = fetchGet(u, "");
if (fetch == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno));
return 2;
kevansUnsubmitted
Done
Inline Actions

Leaks file

kevans: Leaks `file`
}
int chunk_size = 4096;
kevansUnsubmitted
Done
Inline Actions

Ditto for these declarations; chunk_size I'd probably just make a #define since we're not going to be altering it

kevans: Ditto for these declarations; chunk_size I'd probably just make a `#define` since we're not…
char buf[chunk_size];
size_t bytes;
while ((bytes = fread(buf, 1, chunk_size, fetch)) > 0) {
if (fwrite(buf, 1, bytes, file) != bytes) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to write to file: %s", strerror(errno));
fclose(file);
kevansUnsubmitted
Done
Inline Actions

Leaks fetch

kevans: Leaks `fetch`
dchAuthorUnsubmitted
Done
Inline Actions

If I fclose(fetch); here, this segfaults:

Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC-Rio

f.get_url("https://invalid.site/", "/tmp/i")

fish: Job 1, '/usr/libexec/flua $argv' terminated by signal SIGSEGV (Address boundary error)

(lldb) thr backtrace all
* thread #1, name = 'flua', stop reason = signal SIGSEGV
  * frame #0: 0x00001252697ad0f9
    frame #1: 0xffadb0abacff9a89
dch: If I `fclose(fetch);` here, this segfaults: Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC…
kevansUnsubmitted
Done
Inline Actions

The context for this one has changed, the comment was originally down in the error path in the loop. This branch looks fine

kevans: The context for this one has changed, the comment was originally down in the error path in the…
return 2;
}
}
fclose(fetch);
fclose(file);
fetchFreeURL(u);
lua_pushboolean(L, 1);
return 1;
}
static const struct
luaL_Reg fetchlib[] = {
{"parse_url", lfetch_parse_url},
{"get_url", lfetch_get_url},
{NULL, NULL}
};
int
luaopen_fetch(lua_State *L)
{
luaL_newlib(L, fetchlib);
return (1);
}
View Options

sys/dev/nvme/nvme_ns.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2023 Dave Cottlehuber <dch@FreeBSD.org>
*
* 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.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <errno.h>
#include <sys/param.h>
#include <stdio.h>
#include <string.h>
#include <fetch.h>
#include <lua.h>
#include "lauxlib.h"
#include "lfetch.h"
/*
* Minimal implementation of libfetch
*/
static int
lfetch_parse_url(lua_State *L)
{
const char *url = luaL_checkstring(L, 1);
struct url *u;
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
lua_newtable(L);
lua_pushstring(L, u->scheme);
lua_setfield(L, -2, "scheme");
lua_pushstring(L, u->user);
lua_setfield(L, -2, "user");
lua_pushstring(L, u->pwd);
lua_setfield(L, -2, "password");
lua_pushstring(L, u->host);
lua_setfield(L, -2, "host");
/* if port is not explicitly set, it defaults to 0, not scheme */
kevansUnsubmitted
Done
Inline Actions

My gut reaction to this is that we probably shouldn't expose the port at all if it's not explicitly set, just leave it nil in this table to allow for more idiomatic usage where you can simply test the port as truthy before using it.f

kevans: My gut reaction to this is that we probably shouldn't expose the port at all if it's not…
lua_pushinteger(L, u->port);
lua_setfield(L, -2, "port");
/* doc is also known as the query string for http(s) */
lua_pushstring(L, u->doc);
lua_setfield(L, -2, "doc");
fetchFreeURL(u);
return (1);
}
static int
lfetch_get_url(lua_State *L) {
kevansUnsubmitted
Done
Inline Actions

Brace on its own line

kevans: Brace on its own line
const char *url = luaL_checkstring(L, 1);
const char *out = luaL_checkstring(L, 2);
struct url *u;
kevansUnsubmitted
Done
Inline Actions

This should move up above the blank line

kevans: This should move up above the blank line
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
FILE *file = fopen(out, "wb");
kevansUnsubmitted
Done
Inline Actions

IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of this function.

kevans: IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of…
if (file == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to open output file: %s", strerror(errno));
return 2;
}
FILE *fetch = fetchGet(u, "");
if (fetch == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno));
return 2;
kevansUnsubmitted
Done
Inline Actions

Leaks file

kevans: Leaks `file`
}
int chunk_size = 4096;
kevansUnsubmitted
Done
Inline Actions

Ditto for these declarations; chunk_size I'd probably just make a #define since we're not going to be altering it

kevans: Ditto for these declarations; chunk_size I'd probably just make a `#define` since we're not…
char buf[chunk_size];
size_t bytes;
while ((bytes = fread(buf, 1, chunk_size, fetch)) > 0) {
if (fwrite(buf, 1, bytes, file) != bytes) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to write to file: %s", strerror(errno));
fclose(file);
kevansUnsubmitted
Done
Inline Actions

Leaks fetch

kevans: Leaks `fetch`
dchAuthorUnsubmitted
Done
Inline Actions

If I fclose(fetch); here, this segfaults:

Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC-Rio

f.get_url("https://invalid.site/", "/tmp/i")

fish: Job 1, '/usr/libexec/flua $argv' terminated by signal SIGSEGV (Address boundary error)

(lldb) thr backtrace all
* thread #1, name = 'flua', stop reason = signal SIGSEGV
  * frame #0: 0x00001252697ad0f9
    frame #1: 0xffadb0abacff9a89
dch: If I `fclose(fetch);` here, this segfaults: Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC…
kevansUnsubmitted
Done
Inline Actions

The context for this one has changed, the comment was originally down in the error path in the loop. This branch looks fine

kevans: The context for this one has changed, the comment was originally down in the error path in the…
return 2;
}
}
fclose(fetch);
fclose(file);
fetchFreeURL(u);
lua_pushboolean(L, 1);
return 1;
}
static const struct
luaL_Reg fetchlib[] = {
{"parse_url", lfetch_parse_url},
{"get_url", lfetch_get_url},
{NULL, NULL}
};
int
luaopen_fetch(lua_State *L)
{
luaL_newlib(L, fetchlib);
return (1);
}