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.

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

usb: unify handling of length parameters #177

Merged
merged 3 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 0 additions & 2 deletions board-support/raspberrypi-rp2040/src/hal/usb.zig
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ pub const utf8ToUtf16Le = usb.utf8Toutf16Le;

pub var EP0_OUT_CFG: usb.EndpointConfiguration = .{
.descriptor = &usb.EndpointDescriptor{
.length = @as(u8, @intCast(@sizeOf(usb.EndpointDescriptor))),
.descriptor_type = usb.DescType.Endpoint,
.endpoint_address = usb.EP0_OUT_ADDR,
.attributes = @intFromEnum(usb.TransferType.Control),
Expand All @@ -57,7 +56,6 @@ pub var EP0_OUT_CFG: usb.EndpointConfiguration = .{

pub var EP0_IN_CFG: usb.EndpointConfiguration = .{
.descriptor = &usb.EndpointDescriptor{
.length = @as(u8, @intCast(@sizeOf(usb.EndpointDescriptor))),
.descriptor_type = usb.DescType.Endpoint,
.endpoint_address = usb.EP0_IN_ADDR,
.attributes = @intFromEnum(usb.TransferType.Control),
Expand Down
18 changes: 9 additions & 9 deletions core/src/core/usb.zig
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ pub const Dir = enum(u8) {
/// Describes an endpoint within an interface
pub const EndpointDescriptor = extern struct {
/// Length of this struct, must be 7.
length: u8,
length: u8 = 7,
/// Type of this descriptor, must be `Endpoint`.
descriptor_type: DescType,
/// Address of this endpoint, where the bottom 4 bits give the endpoint
Expand All @@ -530,7 +530,7 @@ pub const EndpointDescriptor = extern struct {

pub fn serialize(self: *const @This()) [7]u8 {
var out: [7]u8 = undefined;
out[0] = 7; // length
out[0] = self.length;
Copy link
Contributor

Choose a reason for hiding this comment

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

This function should assert self.length to be 7, and hardcode it with either @sizeOf(@This()) or use out.len.

Otherwise, a user could pass in a bad self which has length = 6 or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That a pretty good idea, will implement it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only out.len is possible as the size is reported with padding, which is not correct.

out[1] = @intFromEnum(self.descriptor_type);
out[2] = self.endpoint_address;
out[3] = self.attributes;
Expand All @@ -544,7 +544,7 @@ pub const EndpointDescriptor = extern struct {
/// Description of an interface within a configuration.
pub const InterfaceDescriptor = extern struct {
/// Length of this structure, must be 9.
length: u8,
length: u8 = 9,
/// Type of this descriptor, must be `Interface`.
descriptor_type: DescType,
/// ID of this interface.
Expand All @@ -566,7 +566,7 @@ pub const InterfaceDescriptor = extern struct {

pub fn serialize(self: *const @This()) [9]u8 {
var out: [9]u8 = undefined;
out[0] = 9; // length
out[0] = self.length;
out[1] = @intFromEnum(self.descriptor_type);
out[2] = self.interface_number;
out[3] = self.alternate_setting;
Expand All @@ -582,7 +582,7 @@ pub const InterfaceDescriptor = extern struct {
/// Description of a single available device configuration.
pub const ConfigurationDescriptor = extern struct {
/// Length of this structure, must be 9.
length: u8,
length: u8 = 9,
/// Type of this descriptor, must be `Config`.
descriptor_type: DescType,
/// Total length of all descriptors in this configuration, concatenated.
Expand Down Expand Up @@ -610,7 +610,7 @@ pub const ConfigurationDescriptor = extern struct {

pub fn serialize(self: *const @This()) [9]u8 {
var out: [9]u8 = undefined;
out[0] = 9; // length
out[0] = self.length;
out[1] = @intFromEnum(self.descriptor_type);
out[2] = @intCast(self.total_length & 0xff);
out[3] = @intCast((self.total_length >> 8) & 0xff);
Expand All @@ -627,7 +627,7 @@ pub const ConfigurationDescriptor = extern struct {
/// typically the first thing the host asks for.
pub const DeviceDescriptor = extern struct {
/// Length of this structure, must be 18.
length: u8,
length: u8 = 18,
/// Type of this descriptor, must be `Device`.
descriptor_type: DescType,
/// Version of the device descriptor / USB protocol, in binary-coded
Expand Down Expand Up @@ -658,7 +658,7 @@ pub const DeviceDescriptor = extern struct {

pub fn serialize(self: *const @This()) [18]u8 {
var out: [18]u8 = undefined;
out[0] = 18; // length
out[0] = self.length;
out[1] = @intFromEnum(self.descriptor_type);
out[2] = @intCast(self.bcd_usb & 0xff);
out[3] = @intCast((self.bcd_usb >> 8) & 0xff);
Expand Down Expand Up @@ -705,7 +705,7 @@ pub const DeviceQualifierDescriptor = extern struct {

pub fn serialize(self: *const @This()) [10]u8 {
var out: [10]u8 = undefined;
out[0] = 10; // length
out[0] = self.length;
out[1] = @intFromEnum(self.descriptor_type);
out[2] = @intCast(self.bcd_usb & 0xff);
out[3] = @intCast((self.bcd_usb >> 8) & 0xff);
Expand Down
2 changes: 1 addition & 1 deletion core/src/core/usb/hid.zig
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub const HidDescriptor = extern struct {

pub fn serialize(self: *const @This()) [9]u8 {
var out: [9]u8 = undefined;
out[0] = 9; // length
out[0] = self.length;
out[1] = @intFromEnum(self.descriptor_type);
out[2] = @intCast(self.bcd_hid & 0xff);
out[3] = @intCast((self.bcd_hid >> 8) & 0xff);
Expand Down
5 changes: 0 additions & 5 deletions examples/raspberrypi-rp2040/src/usb_device.zig
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ fn ep1_out_callback(dc: *usb.DeviceConfiguration, data: []const u8) void {
// add your own endpoints to...
pub var EP1_OUT_CFG: usb.EndpointConfiguration = .{
.descriptor = &usb.EndpointDescriptor{
.length = @as(u8, @intCast(@sizeOf(usb.EndpointDescriptor))),
.descriptor_type = usb.DescType.Endpoint,
.endpoint_address = usb.Dir.Out.endpoint(1),
.attributes = @intFromEnum(usb.TransferType.Bulk),
Expand All @@ -55,7 +54,6 @@ pub var EP1_OUT_CFG: usb.EndpointConfiguration = .{

pub var EP1_IN_CFG: usb.EndpointConfiguration = .{
.descriptor = &usb.EndpointDescriptor{
.length = @as(u8, @intCast(@sizeOf(usb.EndpointDescriptor))),
.descriptor_type = usb.DescType.Endpoint,
.endpoint_address = usb.Dir.In.endpoint(1),
.attributes = @intFromEnum(usb.TransferType.Bulk),
Expand All @@ -73,7 +71,6 @@ pub var EP1_IN_CFG: usb.EndpointConfiguration = .{
// This is our device configuration
pub var DEVICE_CONFIGURATION: usb.DeviceConfiguration = .{
.device_descriptor = &.{
.length = @as(u8, @intCast(@sizeOf(usb.DeviceDescriptor))),
.descriptor_type = usb.DescType.Device,
.bcd_usb = 0x0110,
.device_class = 0,
Expand All @@ -89,7 +86,6 @@ pub var DEVICE_CONFIGURATION: usb.DeviceConfiguration = .{
.num_configurations = 1,
},
.interface_descriptor = &.{
.length = @as(u8, @intCast(@sizeOf(usb.InterfaceDescriptor))),
.descriptor_type = usb.DescType.Interface,
.interface_number = 0,
.alternate_setting = 0,
Expand All @@ -101,7 +97,6 @@ pub var DEVICE_CONFIGURATION: usb.DeviceConfiguration = .{
.interface_s = 0,
},
.config_descriptor = &.{
.length = @as(u8, @intCast(@sizeOf(usb.ConfigurationDescriptor))),
.descriptor_type = usb.DescType.Config,
.total_length = @as(u8, @intCast(@sizeOf(usb.ConfigurationDescriptor) + @sizeOf(usb.InterfaceDescriptor) + @sizeOf(usb.EndpointDescriptor) + @sizeOf(usb.EndpointDescriptor))),
.num_interfaces = 1,
Expand Down
5 changes: 0 additions & 5 deletions examples/raspberrypi-rp2040/src/usb_hid.zig
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ fn ep1_out_callback(dc: *usb.DeviceConfiguration, data: []const u8) void {
// add your own endpoints to...
pub var EP1_OUT_CFG: usb.EndpointConfiguration = .{
.descriptor = &usb.EndpointDescriptor{
.length = @as(u8, @intCast(@sizeOf(usb.EndpointDescriptor))),
.descriptor_type = usb.DescType.Endpoint,
.endpoint_address = usb.Dir.Out.endpoint(1),
.attributes = @intFromEnum(usb.TransferType.Interrupt),
Expand All @@ -55,7 +54,6 @@ pub var EP1_OUT_CFG: usb.EndpointConfiguration = .{

pub var EP1_IN_CFG: usb.EndpointConfiguration = .{
.descriptor = &usb.EndpointDescriptor{
.length = @as(u8, @intCast(@sizeOf(usb.EndpointDescriptor))),
.descriptor_type = usb.DescType.Endpoint,
.endpoint_address = usb.Dir.In.endpoint(1),
.attributes = @intFromEnum(usb.TransferType.Interrupt),
Expand All @@ -73,7 +71,6 @@ pub var EP1_IN_CFG: usb.EndpointConfiguration = .{
// This is our device configuration
pub var DEVICE_CONFIGURATION: usb.DeviceConfiguration = .{
.device_descriptor = &.{
.length = @as(u8, @intCast(@sizeOf(usb.DeviceDescriptor))),
.descriptor_type = usb.DescType.Device,
.bcd_usb = 0x0200,
.device_class = 0,
Expand All @@ -91,7 +88,6 @@ pub var DEVICE_CONFIGURATION: usb.DeviceConfiguration = .{
.num_configurations = 1,
},
.interface_descriptor = &.{
.length = @as(u8, @intCast(@sizeOf(usb.InterfaceDescriptor))),
.descriptor_type = usb.DescType.Interface,
.interface_number = 0,
.alternate_setting = 0,
Expand All @@ -103,7 +99,6 @@ pub var DEVICE_CONFIGURATION: usb.DeviceConfiguration = .{
.interface_s = 0,
},
.config_descriptor = &.{
.length = @as(u8, @intCast(@sizeOf(usb.ConfigurationDescriptor))),
.descriptor_type = usb.DescType.Config,
.total_length = @as(u8, @intCast(@sizeOf(usb.ConfigurationDescriptor) + @sizeOf(usb.InterfaceDescriptor) + @sizeOf(usb.EndpointDescriptor) + @sizeOf(usb.EndpointDescriptor))),
.num_interfaces = 1,
Expand Down